lkml.org 
[lkml]   [2008]   [Sep]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/3] 24-bit types: typedef and macros for accessing 3-byte arrays as integers
    Dave Kleikamp wrote:
    > On Wed, 2008-09-10 at 19:11 +0300, Boaz Harrosh wrote:
    >> Dave Kleikamp wrote:
    >
    >>> @@ -62,7 +60,7 @@ struct timestruc_t {
    >>> */
    >>> typedef struct {
    >>> unsigned len:24;
    >>> - unsigned off1:8;
    >>> + u8 off1;
    >>> u32 off2;
    >>> } lxd_t;
    >>>
    >> Why is the difference from below definition. That is the
    >> use/not of __le24?
    >
    > Answered elsewhere, but this is host-endian. I plan to kill this
    > structure soon.
    >
    >>> @@ -90,8 +88,8 @@ struct lxdlist {
    >>> * physical xd (pxd)
    >>> */
    >>> typedef struct {
    >>> - unsigned len:24;
    >>> - unsigned addr1:8;
    >>> + __le24 len;
    >> Is this stuff on-the-wire?
    >
    > Written to disk, so basically, yeah.
    >
    >> Do you need a:
    >> + __le24 len __packed;
    >>
    >>> + u8 addr1;
    >>> __le32 addr2;
    >>> } pxd_t;
    >> and:
    >> } pxd_t __packed ;
    >
    > I'm not convinced that this is needed. Does the compiler do any padding
    > for alignment when it only contains char types (or structs of chars)?
    >
    >> Note that before the :24 bit-field was kept packed but now
    >> with the use of struct at the __le24 definition it might
    >> choose to pad them.
    >
    > Maybe, but I can't get the compiler to add any padding playing around
    > with variants of these structures. I've tested a simple program on both
    > x86 and ppc64, but I'm not sure what would happen on, say, arm.
    >

    You have an "__le32 addr2" followed, it might want too in rare cases.
    But for me this is also a a declaration issue and a readability issue.

    I'm telling the compiler, don't mess with my structure, this needs
    to be constant whatever the compiler is. In C the compiler is even
    allowed to change fields order if it wants to. Why the guess work,
    __packed and be done with it.

    And it's also a readability issue. Look above lxd_t vs pxd_t. I can't
    know that one is in memory and one is on disk. I have to ask questions
    and make wrong remarks. But if the later had a __packed on it, then
    there are no more questions.

    __packed for me is a statement for both the programmer and compiler
    that says: "This stuff will be seen externally of the machine. It must
    be universally constant"

    >> Chris you might want to change the definitions at linux/types.h
    >> to:
    >>
    >> typedef struct { __u8 b[3]; } __be24, __le24 __packed;
    >>
    >> With gcc it will not help with the proceeding fields, and the
    >> containing struct will need it's own "__packed" declaration
    >> but it will keep it packed with previous fields.
    >>
    >> Just my $0.017
    >> Boaz
    >
    > Shaggy

    Boaz


    \
     
     \ /
      Last update: 2008-09-11 10:41    [W:0.027 / U:238.420 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site