Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c > #8641 > unrolled thread

offsetOf

Back to article view | Back to comp.lang.c

Contents

  offsetOf Jack Boot <jackboot@mailinator.com> - 2011-07-23 20:54 +0000
    Re: offsetOf Ian Collins <ian-news@hotmail.com> - 2011-07-24 09:05 +1200
    Re: offsetOf Ben Bacarisse <ben.usenet@bsb.me.uk> - 2011-07-23 23:03 +0100
      Re: offsetOf Jack Boot <jackboot@mailinator.com> - 2011-07-24 10:30 +0000
        Re: offsetOf Ian Collins <ian-news@hotmail.com> - 2011-07-24 22:41 +1200
        Re: offsetOf James Kuyper <jameskuyper@verizon.net> - 2011-07-24 08:36 -0400
        Re: offsetOf jt@toerring.de (Jens Thoms Toerring) - 2011-07-24 14:50 +0000
        Re: offsetOf Joe Pfeiffer <pfeiffer@cs.nmsu.edu> - 2011-07-24 10:28 -0600
        Re: offsetOf Nobody <nobody@nowhere.com> - 2011-07-24 17:30 +0100
        Re: offsetOf gordonb.gkpc8@burditt.org (Gordon Burditt) - 2011-07-27 08:43 -0500
      Re: offsetOf Phil Carmody <thefatphil_demunged@yahoo.co.uk> - 2011-07-24 14:18 +0300
        Re: offsetOf Ben Bacarisse <ben.usenet@bsb.me.uk> - 2011-07-24 12:50 +0100
          Re: offsetOf Phil Carmody <thefatphil_demunged@yahoo.co.uk> - 2011-07-24 18:25 +0300
    Re: offsetOf Eric Sosman <esosman@ieee-dot-org.invalid> - 2011-07-23 20:12 -0400
    Re: offsetOf "J. J. Farrell" <jjf@bcs.org.uk> - 2011-07-24 01:57 +0100

#8641 — offsetOf

Hello:

This came up in a code review recently, I've never heard of it before, 
opinions welcome.

There is a struct that looks a bit like this

struct foo {
  int a[10];
  long b;
  ...
};

Now I have a function that starts thusly:

void
bar ( struct foo * f )
{
  int * ip = (int *) foo;
  ...
}

The reviewer recommended, that for maximal portability I should replace 
this by:

int * ip = ( (int *) ( ( (unsigned char *) foo ) + offsetOf(struct foo, 
a) ) );

I've never seen this construction before, also I think type punning is 
inherently non-portable so I'm not sure how much this buys you.

Cheers,
JB

[toc] | [next] | [standalone]

#8642

On 07/24/11 08:54 AM, Jack Boot wrote:
> Hello:
>
> This came up in a code review recently, I've never heard of it before,
> opinions welcome.
>
> There is a struct that looks a bit like this
>
> struct foo {
>    int a[10];
>    long b;
>    ...
> };
>
> Now I have a function that starts thusly:
>
> void
> bar ( struct foo * f )
> {
>    int * ip = (int *) foo;
>    ...
> }
>
> The reviewer recommended, that for maximal portability I should replace
> this by:
>
> int * ip = ( (int *) ( ( (unsigned char *) foo ) + offsetOf(struct foo,
> a) ) );

Where's the chunder bucket?

Why not

   int* ip = foo->a;

> I've never seen this construction before, also I think type punning is
> inherently non-portable so I'm not sure how much this buys you.

It's a horrible mess no matter how you look at it.  What is ip used for?

-- 
Ian Collins

[toc] | [prev] | [next] | [standalone]

#8644

Jack Boot <jackboot@mailinator.com> writes:

> This came up in a code review recently, I've never heard of it before, 
> opinions welcome.
>
> There is a struct that looks a bit like this
>
> struct foo {
>   int a[10];
>   long b;
>   ...
> };
>
> Now I have a function that starts thusly:
>
> void
> bar ( struct foo * f )
> {
>   int * ip = (int *) foo;
>   ...
> }
>
> The reviewer recommended, that for maximal portability I should replace 
> this by:
>
> int * ip = ( (int *) ( ( (unsigned char *) foo ) + offsetOf(struct foo, 
> a) ) );
>
> I've never seen this construction before, also I think type punning is 
> inherently non-portable so I'm not sure how much this buys you.

Nothing but obfuscation.   offsetOf(struct foo, a) is 0 (it can't be
otherwise) so this just converts to a char * first.  This won't hurt but
it won't help either.  This:

  int *ip = f->a;

is, to my mind, the clearest way to write it.  I can't see why it would
be done with any casts at all.

-- 
Ben.

[toc] | [prev] | [next] | [standalone]

#8656

Ben Bacarisse wrote:

> Jack Boot <jackboot@mailinator.com> writes:
> 
>> This came up in a code review recently, I've never heard of it before,
>> opinions welcome.
>>
>> There is a struct that looks a bit like this
>>
>> struct foo {
>>   int a[10];
>>   long b;
>>   ...
>> };
>>
>> Now I have a function that starts thusly:
>>
>> void
>> bar ( struct foo * f )
>> {
>>   int * ip = (int *) foo;
>>   ...
>> }
>>
>> The reviewer recommended, that for maximal portability I should replace
>> this by:
>>
>> int * ip = ( (int *) ( ( (unsigned char *) foo ) + offsetOf(struct foo,
>> a) ) );
>>
>> I've never seen this construction before, also I think type punning is
>> inherently non-portable so I'm not sure how much this buys you.
> 
> Nothing but obfuscation.   offsetOf(struct foo, a) is 0 (it can't be
> otherwise) so this just converts to a char * first.  This won't hurt but
> it won't help either.  This:
> 
>   int *ip = f->a;
> 
> is, to my mind, the clearest way to write it.  I can't see why it would
> be done with any casts at all.

Thanks for all the replies.

I wrote the code with the cast because I used struct foo as an Opaque Type 
in that file (only using pointers, the header with the struct definition 
not #include'd) to emulate C++-style encapsulation. Therefore I could not 
dereference f to get f->a.

The reviewer was concerned that there might be padding before a. I'm not 
sure any compiler would need to insert padding around int types, though I 
guess it's possible on a 64-bit arch with 32-bit ints. Anyways, to use his 
offsetOf solution you need to include the struct def, so in that case I 
agree with all you guys that f->a would be preferable.

Cheers,
JB

[toc] | [prev] | [next] | [standalone]

#8658

On 07/24/11 10:30 PM, Jack Boot wrote:
> Ben Bacarisse wrote:
>
>> Jack Boot<jackboot@mailinator.com>  writes:
>>
>>> This came up in a code review recently, I've never heard of it before,
>>> opinions welcome.
>>>
>>> There is a struct that looks a bit like this
>>>
>>> struct foo {
>>>    int a[10];
>>>    long b;
>>>    ...
>>> };
>>>
>>> Now I have a function that starts thusly:
>>>
>>> void
>>> bar ( struct foo * f )
>>> {
>>>    int * ip = (int *) foo;
>>>    ...
>>> }
>>>
>>> The reviewer recommended, that for maximal portability I should replace
>>> this by:
>>>
>>> int * ip = ( (int *) ( ( (unsigned char *) foo ) + offsetOf(struct foo,
>>> a) ) );
>>>
>>> I've never seen this construction before, also I think type punning is
>>> inherently non-portable so I'm not sure how much this buys you.
>>
>> Nothing but obfuscation.   offsetOf(struct foo, a) is 0 (it can't be
>> otherwise) so this just converts to a char * first.  This won't hurt but
>> it won't help either.  This:
>>
>>    int *ip = f->a;
>>
>> is, to my mind, the clearest way to write it.  I can't see why it would
>> be done with any casts at all.
>
> Thanks for all the replies.
>
> I wrote the code with the cast because I used struct foo as an Opaque Type
> in that file (only using pointers, the header with the struct definition
> not #include'd) to emulate C++-style encapsulation. Therefore I could not
> dereference f to get f->a.
>
> The reviewer was concerned that there might be padding before a.

There won't be.  As Eric said: A pointer to a struct can be converted to 
a pointer to its first element, and vice versa.

-- 
Ian Collins

[toc] | [prev] | [next] | [standalone]

#8661

On 07/24/2011 06:30 AM, Jack Boot wrote:
...
> I wrote the code with the cast because I used struct foo as an Opaque Type 
> in that file (only using pointers, the header with the struct definition 
> not #include'd) to emulate C++-style encapsulation. Therefore I could not 
> dereference f to get f->a.

When you use opaque types, translation units which work with that type
should fall into two categories:

1. Code which handles the type as opaque. Such code only needs a
declaration of the type of the form:
	struct my_opaque_type;
It's the use of such a declaration that makes this an opaque type.

2. Code which looks inside the type. Such code should have a full
definition of the data type. This definition should be #included from a
single common header file.

The code you're writing is solidly in the second category, but for some
reason you're trying to write it without the full definition of the
type. That's the fundamental mistake; the dubious code you're writing is
a normal consequence of that bad decision, and is in fact the reason why
that decision was a bad one.

> The reviewer was concerned that there might be padding before a.

There cannot be - but that's not the relevant issue. In the future, the
definition of the type might be changed so that something does come
before 'a', and that is a serious problem for your code, but the
something that comes before 'a' has to include at least one real field;
it can't all be padding.

If your code needs to access a, it should have available to it a full
definition for the type that actually includes a. If, for any reason, it
should NOT have that full type definition, it should also not be trying
to access a.
-- 
James Kuyper

[toc] | [prev] | [next] | [standalone]

#8664

Jack Boot <jackboot@mailinator.com> wrote:
> Ben Bacarisse wrote:
> > Jack Boot <jackboot@mailinator.com> writes:
> > 
> >> This came up in a code review recently, I've never heard of it before,
> >> opinions welcome.
> >>
> >> There is a struct that looks a bit like this
> >>
> >> struct foo {
> >>   int a[10];
> >>   long b;
> >>   ...
> >> };
> >>
> >> Now I have a function that starts thusly:
> >>
> >> void
> >> bar ( struct foo * f )
> >> {
> >>   int * ip = (int *) foo;
> >>   ...
> >> }
> >>
> >> The reviewer recommended, that for maximal portability I should replace
> >> this by:
> >>
> >> int * ip = ( (int *) ( ( (unsigned char *) foo ) + offsetOf(struct foo,
> >> a) ) );
> >>
> >> I've never seen this construction before, also I think type punning is
> >> inherently non-portable so I'm not sure how much this buys you.
> > 
> > Nothing but obfuscation.   offsetOf(struct foo, a) is 0 (it can't be
> > otherwise) so this just converts to a char * first.  This won't hurt but
> > it won't help either.  This:
> > 
> >   int *ip = f->a;
> > 
> > is, to my mind, the clearest way to write it.  I can't see why it would
> > be done with any casts at all.

> Thanks for all the replies.

> I wrote the code with the cast because I used struct foo as an Opaque Type 
> in that file (only using pointers, the header with the struct definition 
> not #include'd)

In that case the code using the offsetof() macro can't compile,
since offsetof() needs to know how the structure looks like.
And it doesn't sound like a very useful kind of "encapsulation"
if you then retrieve something from the innards of the structure.

                               Regards, Jens
-- 
  \   Jens Thoms Toerring  ___      jt@toerring.de
   \__________________________      http://toerring.de

[toc] | [prev] | [next] | [standalone]

#8670

Jack Boot <jackboot@mailinator.com> writes:
>
> I wrote the code with the cast because I used struct foo as an Opaque Type 
> in that file (only using pointers, the header with the struct definition 
> not #include'd) to emulate C++-style encapsulation. Therefore I could not 
> dereference f to get f->a.

You're making the struct an opaque type, but then taking advantage of
your knowledge of its internal representation.  You can't really have it
both ways; you should either expose the internal representation using
f->a or else write an accessor function that returns a (or even an
accessor for the elements of a, or a function that's part of your foo
implementation that does whatever manipulation is going on in bar()).

[toc] | [prev] | [next] | [standalone]

#8671

On Sun, 24 Jul 2011 10:30:23 +0000, Jack Boot wrote:

> I wrote the code with the cast because I used struct foo as an Opaque Type
> in that file

No you didn't, you relied upon the fact that the first element of the
structure was an array of integers. That *isn't* using it as an opaque
type.

[toc] | [prev] | [next] | [standalone]

#8776

> I wrote the code with the cast because I used struct foo as an Opaque Type 
> in that file (only using pointers, the header with the struct definition 
> not #include'd) to emulate C++-style encapsulation. Therefore I could not 
> dereference f to get f->a.

No, you're using it as a Not An Opaque Type (tm).  If you want to
access f->a, then provide a type method (over in the type-aware
section of code) to give you the address of the array given the
pointer to struct, or just stop trying to treat a Translucent Type
as an Opaque Type, and write f->a.

> The reviewer was concerned that there might be padding before a. I'm not 

If it's an Opaque Type, you don't know that a is the first element
of a struct.  In fact, you don't know that it's even an element of
the struct at all; you might have to go through several layers of
pointers to get to it.  Part of the point of opaque types is that
you don't have to know such details, and they can be changed without
affecting code that uses the type.

> sure any compiler would need to insert padding around int types, though I 

A compiler is forbidden from adding padding before the first element
of a struct, regardless of that element's type.  However, a maintenance
programmer is not forbidden from adding an element before 'a', and
this would break your code in a non-obvious way.

The compiler might choose to align all structs on 64-bit boundaries
if there were types (like long long, or double) that need to be
aligned on 64-bit boundaries, *whether or not the struct contains
that type*.  Or it might just align the struct to the strictest
alignment of a type actually used in the struct.

> guess it's possible on a 64-bit arch with 32-bit ints. Anyways, to use his 
> offsetOf solution you need to include the struct def, so in that case I 
> agree with all you guys that f->a would be preferable.

[toc] | [prev] | [next] | [standalone]

#8659

Ben Bacarisse <ben.usenet@bsb.me.uk> writes:
> Jack Boot <jackboot@mailinator.com> writes:
> 
> > This came up in a code review recently, I've never heard of it before, 
> > opinions welcome.
> >
> > There is a struct that looks a bit like this
> >
> > struct foo {
> >   int a[10];
> >   long b;
> >   ...
> > };
> >
> > Now I have a function that starts thusly:
> >
> > void
> > bar ( struct foo * f )
> > {
> >   int * ip = (int *) foo;
> >   ...
> > }
> >
> > The reviewer recommended, that for maximal portability I should replace 
> > this by:
> >
> > int * ip = ( (int *) ( ( (unsigned char *) foo ) + offsetOf(struct foo, 
> > a) ) );
> >
> > I've never seen this construction before, also I think type punning is 
> > inherently non-portable so I'm not sure how much this buys you.
> 
> Nothing but obfuscation.

That's kind to it.

> offsetOf(struct foo, a) is 0 (it can't be
> otherwise) so this just converts to a char * first.  This won't hurt but
> it won't help either.  This:
> 
>   int *ip = f->a;

I have this fetish for wanting addresses to explicitly be addresses
(the result of applying the 'address of' operator), so my default 
would be that which explicitly says 'the address of the first element':

  int *ip = &f->a[0];

I'm consistent with this, I also like to pass functions around with 
an explicit '&' before their name.

The weird thing is that I'm usually one who favours brevity, but for 
addresses, I like a clear flag that reminds me what I'm dealing with.

> is, to my mind, the clearest way to write it.  I can't see why it would
> be done with any casts at all.

100% agree. Casts are usually a warning sign.

Phil
-- 
"At least you know where you are with Microsoft."
"True. I just wish I'd brought a paddle." -- Matthew Vernon

[toc] | [prev] | [next] | [standalone]

#8660

Phil Carmody <thefatphil_demunged@yahoo.co.uk> writes:

> Ben Bacarisse <ben.usenet@bsb.me.uk> writes:
>> Jack Boot <jackboot@mailinator.com> writes:
<snip>
>> > struct foo {
>> >   int a[10];
>> >   long b;
>> >   ...
>> > };
<snip>
>> > void
>> > bar ( struct foo * f )
>> > {
<snip>
>> > }
<snip>
>>   int *ip = f->a;
>
> I have this fetish for wanting addresses to explicitly be addresses
> (the result of applying the 'address of' operator), so my default 
> would be that which explicitly says 'the address of the first element':
>
>   int *ip = &f->a[0];
>
> I'm consistent with this,

How many of these are there in your code:

  const char *digits = &"0123456789"[0];
  ...
  strcmp(&"string"[0], str) == 0

?

<snip>
-- 
Ben.

[toc] | [prev] | [next] | [standalone]

#8666

Ben Bacarisse <ben.usenet@bsb.me.uk> writes:
> Phil Carmody <thefatphil_demunged@yahoo.co.uk> writes:
> 
> > Ben Bacarisse <ben.usenet@bsb.me.uk> writes:
> >> Jack Boot <jackboot@mailinator.com> writes:
> <snip>
> >> > struct foo {
> >> >   int a[10];
> >> >   long b;
> >> >   ...
> >> > };
> <snip>
> >> > void
> >> > bar ( struct foo * f )
> >> > {
> <snip>
> >> > }
> <snip>
> >>   int *ip = f->a;
> >
> > I have this fetish for wanting addresses to explicitly be addresses
> > (the result of applying the 'address of' operator), so my default 
> > would be that which explicitly says 'the address of the first element':
> >
> >   int *ip = &f->a[0];
> >
> > I'm consistent with this,
> 
> How many of these are there in your code:
> 
>   const char *digits = &"0123456789"[0];
>   ...
>   strcmp(&"string"[0], str) == 0

A pair of '"'s is another clear indicator, so precisely zero.

Phil
-- 
"At least you know where you are with Microsoft."
"True. I just wish I'd brought a paddle." -- Matthew Vernon

[toc] | [prev] | [next] | [standalone]

#8647

On 7/23/2011 4:54 PM, Jack Boot wrote:
> Hello:
>
> This came up in a code review recently, I've never heard of it before,
> opinions welcome.
>
> There is a struct that looks a bit like this
>
> struct foo {
>    int a[10];
>    long b;
>    ...
> };
>
> Now I have a function that starts thusly:
>
> void
> bar ( struct foo * f )
> {
>    int * ip = (int *) foo;

     I guess you mean `f' here.

>    ...
> }
>
> The reviewer recommended, that for maximal portability I should replace
> this by:
>
> int * ip = ( (int *) ( ( (unsigned char *) foo ) + offsetOf(struct foo,
> a) ) );
>
> I've never seen this construction before, also I think type punning is
> inherently non-portable so I'm not sure how much this buys you.

     I don't think the reviewer is concerned about portability, because
the code as it stands is entirely portable: A pointer to a struct can
be converted to a pointer to its first element, and vice versa, and
nothing will go wrong.

     BUT what happens if somebody adds a new element `x' to the struct
and puts it before `a', so `a' is no longer the first element?  The
code becomes "portably incorrect," and the compiler is unlikely to warn
you about the error.

     If what you need is a pointer to the start of the `a' array, use

	int *ip = f->a;

This construct will find `a' no matter where it sits in the struct --
and has the further virtue that the the compiler will squawk if somebody
changes `a' to an array of `long' instead of `int'.

     If you also require (for some reason not apparent in the code as
shown) that `a' be first, you can

	assert(offsetof(struct foo, a) == 0);

... or use a "compile-time assertion" (GIYF) to make the same test.

-- 
Eric Sosman
esosman@ieee-dot-org.invalid

[toc] | [prev] | [next] | [standalone]

#8648

Jack Boot wrote:
> Hello:
> 
> This came up in a code review recently, I've never heard of it before, 
> opinions welcome.
> 
> There is a struct that looks a bit like this
> 
> struct foo {
>   int a[10];
>   long b;
>   ...
> };
> 
> Now I have a function that starts thusly:
> 
> void
> bar ( struct foo * f )
> {
>   int * ip = (int *) foo;
>   ...
> }
> 
> The reviewer recommended, that for maximal portability I should replace 
> this by:
> 
> int * ip = ( (int *) ( ( (unsigned char *) foo ) + offsetOf(struct foo, 
> a) ) );
> 
> I've never seen this construction before, also I think type punning is 
> inherently non-portable so I'm not sure how much this buys you.

What is "offserOf"? Did they mean "offsetof"? I'll assume so.

Frankly I'd find a reviewer who has a clue and lose this one. Your 
original is valid and portable, though it could arguably be improved in 
style and readability. This proposed improvement is obfuscatory nonsense 
which does nothing.

offsetof(struct foo, a) is always 0, by definition, so we can remove one 
level of obfuscation. This gets us to:

int * ip = ( (int *) ( ( (unsigned char *) foo ) + 0 ) );

which simplifies after removal of pointless bits and obfuscatory 
redundant brackets to

int * ip = (int *)(unsigned char *)foo;

which is a silly way of writing

int * ip = (int *)foo;

which means it is no different from your version, apart from taking a 
lot more typing and being a lot more prone to misunderstanding.

My comment on your code would have been along the lines of:

Why the cast? It's best if casts are only used where absolutely 
necessary, usually indicating something which is non-portable or 
otherwise dodgy. What you're doing here is entirely portable and valid, 
so best to do it cleanly without a cast. You want ip to point to the int 
which you know is at the start of array a which is at the beginning of 
struct foo, so why not just

int *ip = foo->a;

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.c

csiph-web