Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 29 Aug 2017 14:41:11 -0500
From:      Justin Hibbits <chmeeedalf@gmail.com>
To:        Maxim Sobolev <sobomax@sippysoft.com>
Cc:        John Baldwin <jhb@freebsd.org>, svn-src-head@freebsd.org, svn-src-all@freebsd.org,  src-committers <src-committers@freebsd.org>, Ryan Libby <rlibby@freebsd.org>
Subject:   Re: svn commit: r322969 - in head: sbin/mdconfig sys/dev/md sys/sys
Message-ID:  <CAHSQbTC1_fhsC-dwEyLvdU9TnaWoYhOtdpQStw6JNCPbuTsoBw@mail.gmail.com>
In-Reply-To: <CAH7qZfvGL-UgZX5VzZXB%2B6zPznA9-GvEg%2B=X2roiTSUUj5jxSA@mail.gmail.com>
References:  <201708281554.v7SFs8fr014268@repo.freebsd.org> <2937323.CvTEtZnL2T@ralph.baldwin.cx> <CAH7qZftFAzfsgfgHV2=59Ah2LPfMC_CFYUEfiFZSWbf7jMZUTg@mail.gmail.com> <6350259.n2rmZ9RnEY@ralph.baldwin.cx> <CAH7qZfvGL-UgZX5VzZXB%2B6zPznA9-GvEg%2B=X2roiTSUUj5jxSA@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Aug 29, 2017 14:18, "Maxim Sobolev" <sobomax@sippysoft.com> wrote:

John, OK, maybe you are right and the current status quo was just an
accident. I am curious what do you and other people think about expressing
expected structure size and padding more explicitly instead of trying to
accommodate for sometimes intricate play between alignment and type size
with something like char[N]? I.e. along the following lines:

#if __WORDSIZE < 64
#define MD_IOCTL_LEN    436
#else
#define MD_IOCTL_LEN    448
#endif

struct md_ioctl {
    union {
        struct _md_ioctl_payload {
            unsigned        version;     /* Structure layout version */
            unsigned        unit;        /* unit number */
            enum md_types   type ;       /* type of disk */
            char            *file;       /* pathname of file to mount */
            off_t           mediasize;   /* size of disk in bytes */
            unsigned        sectorsize;  /* sectorsize */
            unsigned        options;     /* options */
            u_int64_t       base;        /* base address */
            int             fwheads;     /* firmware heads */
            int             fwsectors;   /* firmware sectors */
            char            *label;      /* label of the device */
        } md;
        char raw[MD_IOCTL_LEN]; /* payload + padding for future ideas */
    };
};
CTASSERT(sizeof(struct md_ioctl) == MD_IOCTL_LEN);

I've tested that it DTRT on i386 and amd64, need to validate on ARM and
MIPS. The code impact should be minimal and easy to apply, i.e. ptr->md_ywz
becomes ptr->md.xyz.


I like this. Being explicit gives us better guarantees when it comes time
to break ABI, rather than it being an accident.

- Justin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAHSQbTC1_fhsC-dwEyLvdU9TnaWoYhOtdpQStw6JNCPbuTsoBw>