Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > Is this struct definition right?

Reply
Thread Tools

Is this struct definition right?

 
 
Keith Thompson
Guest
Posts: n/a
 
      05-28-2013
Anand Hariharan <(E-Mail Removed)> writes:
> On May 25, 8:11*pm, Keith Thompson <(E-Mail Removed)> wrote:
>> Eric Sosman <(E-Mail Removed)> writes:
>>
>> [...]
>>
>> > * * *Meanwhile, *I'm* puzzled by one feature of the code you show,
>> > and hope the experts will chime in. *O Experts: Since lines 24, 26,
>> > 27, and 29 mention `struct pci_dev*' in the function prototypes,
>> > do these functions actually accept pointers to the type declared
>> > in line 33? *Absent any earlier `struct pci_dev' declaration,
>> > aren't the others at "function prototype scope?"

>>
>> Yes, that is puzzling. *There is a full definition of "struct
>> pci_dev" in linux/include/linux/pci.h. *My best guess is that the
>> full definition is made visible by the "#include <linux/compiler.h>"
>> near the top of the file -- but in that case the "forward"
>> declaration would be unnecessary.
>>
>> I've found the commit where the declaration was added; I'll send an
>> e-mail to the authors (or approvers) of that commmit and ask about it.

>
> Are you referring to this or some other revision:
>
> https://github.com/torvalds/linux/bl...pcie/portdrv.h


That's the one.

> Initially it appeared to me that the function prototypes were added
> more than two years after the tentative declaration. A little more
> attention showed that it was only modification (removing the "extern")
> to already existing prototypes, and the so called "forward"
> declaration was added 'later' (pun intended).


Yes, the prototypes were there before the "struct pci_dev;" declaration
was added. Which suggests either that the "struct pci_dev;" declaration
was unnecessary (that's my guess), or that it was made necessary by some
other change -- though the latter wouldn't explain why the declaration
was added *after* the prototypes. That commit affected 17 files.

--
Keith Thompson (The_Other_Keith) http://www.velocityreviews.com/forums/(E-Mail Removed) <http://www.ghoti.net/~kst>
Working, but not speaking, for JetHead Development, Inc.
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
 
Reply With Quote
 
 
 
 
Tim Rentsch
Guest
Posts: n/a
 
      06-12-2013
Malcolm McLean <(E-Mail Removed)> writes:

> On Saturday, May 25, 2013 8:00:42 PM UTC+1, fl wrote:
>>
>> I read the following, which is part of Linux/drivers/pci/pcie/portdrv.h. I
>> do not understand the last definition. It seems there is one parameter
>> missing, i.e.
>>
>> struct pci_dev real_struct_variable;
>>
>> 33 struct pci_dev;

>
> It's a tentative forward declaration. It reserves the structure
> name pci_dev, without actually declaring it. So you can declare
> pointers to it and if you try to redefine it the program will break.
> But you don't know what members it has.


There is no such thing as a "tentative" declaration, or
"forward" declaration either as far ISO C is concerned.
This line is simply a declaration - it declares a type
whose struct tag is 'pci_dev' (and if no other previous
declaration of the same type is visible "creates" it),
so any subsequent references to a type whose struct tag
is 'pci_dev' use the type declared here rather than
accidentally declaring a new one (which would necessarily
have a more limited scope), as might happen if there were
no previous declaration.

The phrase you're looking for is this declaration does
not _define the contents of_ the type, ie, what members
the struct contains. But in terms of declaring the type
this is just a regular declaration.
 
Reply With Quote
 
 
 
 
Tim Rentsch
Guest
Posts: n/a
 
      06-12-2013
Keith Thompson <(E-Mail Removed)> writes:

> (I'm not criticizing you for being wrong; it happens to all of us.)


It does, but it happens to some people a lot more
than others.
 
Reply With Quote
 
Tim Rentsch
Guest
Posts: n/a
 
      06-12-2013
Bart van Ingen Schenau <(E-Mail Removed)> writes:

> On Sat, 25 May 2013 18:11:53 -0700, Keith Thompson wrote:
>
>> Eric Sosman <(E-Mail Removed)> writes: [...]
>>> Meanwhile, *I'm* puzzled by one feature of the code you show,
>>> and hope the experts will chime in. O Experts: Since lines 24, 26, 27,
>>> and 29 mention `struct pci_dev*' in the function prototypes, do these
>>> functions actually accept pointers to the type declared in line 33?
>>> Absent any earlier `struct pci_dev' declaration, aren't the others at
>>> "function prototype scope?"

>>
>> Yes, that is puzzling. There is a full definition of "struct pci_dev"
>> in linux/include/linux/pci.h. My best guess is that the full definition
>> is made visible by the "#include <linux/compiler.h>" near the top of the
>> file -- but in that case the "forward" declaration would be unnecessary.

>
> My guess is that they rely on the type compatibility rules. If the
> definition of struct pci_dev (also) can be found in a different
> translation unit, then clause 6.2.7/1 (C99) stipulates that each of
> the prototype-scope declarations of struct pci_dev is individually
> compatible with the definition in the other TU on the grounds that
> they occur in different TU's and have the same tag.


A little thought should convince you that this isn't right. If
there is no previous declaration of struct pci_dev, then the
parameters of the preceding functions have types that cannot be
satisfied by any other struct pointer type; arguments other than
null pointer constants or (void *) would be violate a constraint.
Of course it's possible that the compiler is run in a way so
that the constraint violation is ignorable, and the developers
do in fact ignore it, but that isn't really very likely.

> It is not a stretch to conjecture that if several declarations are
> each individually compatible with an additional declaration in a
> different TU, that they are also compatible with each other, even
> though this is not backed up by the standard.


The problem is not compatibility with other TU's, the problem
is the constraint violations that occur within a single TU
because of the (putative) multiple, non-compatible types
involved.
 
Reply With Quote
 
Tim Rentsch
Guest
Posts: n/a
 
      06-12-2013
Eric Sosman <(E-Mail Removed)> writes:

> Meanwhile, *I'm* puzzled by one feature of the code you
> show, and hope the experts will chime in. O Experts: Since
> lines 24, 26, 27, and 29 mention `struct pci_dev*' in the
> function prototypes, do these functions actually accept
> pointers to the type declared in line 33? Absent any earlier
> `struct pci_dev' declaration, aren't the others at "function
> prototype scope?"


Yes, they are. Moreover, since they are, any attempt to
supply an argument to one of these functions (and assuming
there is no earlier declaration of struct pci_dev) will
result in a constraint violation (not counting the obvious
circumstances of giving a null pointer constant or an
expression of type (void *) as an argument). So in all
likelihood any TU that includes this file either has a
previous declaration of struct pci_dev or does not call
these functions.
 
Reply With Quote
 
Bart van Ingen Schenau
Guest
Posts: n/a
 
      06-13-2013
On Wed, 12 Jun 2013 15:24:50 -0700, Tim Rentsch wrote:

> Eric Sosman <(E-Mail Removed)> writes:
>
>> Meanwhile, *I'm* puzzled by one feature of the code you
>> show, and hope the experts will chime in. O Experts: Since lines 24,
>> 26, 27, and 29 mention `struct pci_dev*' in the function prototypes, do
>> these functions actually accept pointers to the type declared in line
>> 33? Absent any earlier `struct pci_dev' declaration, aren't the others
>> at "function prototype scope?"

>
> Yes, they are. Moreover, since they are, any attempt to supply an
> argument to one of these functions (and assuming there is no earlier
> declaration of struct pci_dev) will result in a constraint violation
> (not counting the obvious circumstances of giving a null pointer
> constant or an expression of type (void *) as an argument). So in all
> likelihood any TU that includes this file either has a previous
> declaration of struct pci_dev or does not call these functions.


Or, as a third possibility, the compiler (settings) used to compile the
software is non-conforming in that it does not diagnose the constraint
violation.

Bart v Ingen Schenau
 
Reply With Quote
 
Tim Rentsch
Guest
Posts: n/a
 
      06-13-2013
Keith Thompson <(E-Mail Removed)> writes:

> Anand Hariharan <(E-Mail Removed)> writes:
>> On May 25, 8:11 pm, Keith Thompson <(E-Mail Removed)> wrote:
>>> Eric Sosman <(E-Mail Removed)> writes:
>>>
>>> [...]
>>>
>>> > Meanwhile, *I'm* puzzled by one feature of the code you show,
>>> > and hope the experts will chime in. O Experts: Since lines 24, 26,
>>> > 27, and 29 mention `struct pci_dev*' in the function prototypes,
>>> > do these functions actually accept pointers to the type declared
>>> > in line 33? Absent any earlier `struct pci_dev' declaration,
>>> > aren't the others at "function prototype scope?"
>>>
>>> Yes, that is puzzling. There is a full definition of "struct
>>> pci_dev" in linux/include/linux/pci.h. My best guess is that the
>>> full definition is made visible by the "#include <linux/compiler.h>"
>>> near the top of the file -- but in that case the "forward"
>>> declaration would be unnecessary.
>>>
>>> I've found the commit where the declaration was added; I'll send an
>>> e-mail to the authors (or approvers) of that commmit and ask about it.

>>
>> Are you referring to this or some other revision:
>>
>> https://github.com/torvalds/linux/bl...pcie/portdrv.h

>
> That's the one.
>
>> Initially it appeared to me that the function prototypes were
>> added more than two years after the tentative declaration. A
>> little more attention showed that it was only modification
>> (removing the "extern") to already existing prototypes, and the
>> so called "forward" declaration was added 'later' (pun intended).

>
> Yes, the prototypes were there before the "struct pci_dev;"
> declaration was added. Which suggests either that the "struct
> pci_dev;" declaration was unnecessary (that's my guess), or that
> it was made necessary by some other change -- though the latter
> wouldn't explain why the declaration was added *after* the
> prototypes. That commit affected 17 files.


Here is a plausible (and also I think reasonably likely)
explanation.

The file portdrv.h was included in two kinds of circumstances:
one, where the declared functions were called, but the TU did an
include of pci.h before the include of portdrv.h, so the earlier
declarations in portdrv.h were okay; and two, where there was
no earlier include of pci.h, but the offending declarations were
not made use of, so it didn't matter that there was no earlier
declaration of struct pci_dev -- everything compiled fine, and
it all worked because the funny declarations weren't used in
places where they would have done something bad.

At some point, the file portdrv.h was modified to have some
definitions of inline functions. These function bodies make use
of some of the earlier declarations using struct pci_dev. This
caused problems in those TU's where there was no earlier include
for pci.h -- even though they didn't use any of the offending
declarations themselves, the newly added inline definitions in
portdrv.h do, and that caused compilation errors. These errors
can be fixed (I believe -- I haven't tried compiling myself) by
adding the 'struct pci_dev;' declaration where it appears in
portdrv.h. The declaration was put in, the errors disappeared,
and that's how it was left; since everything was compiling and
working okay, the curious placement of the 'struct pci_dev;'
declaration was just left where it was.

Evidence to support this suggestion: AFAICT the inline function
definitions and the 'struct pci_dev;' declaration appeared in the
same revision of portdrv.h. Since the inline function definitions
use some (but only some!) of the earlier declarations, it seems
like a good guess that portdrv.h had been included in some places
where the lack of an earlier declaration of struct pci_dev didn't
matter, but now because of the just-added inline function bodies
it did. And it's easy to imagine that someone added the line with
the declaration of struct pci_dev just early enough to resolve the
problems caused by those new function bodies.
 
Reply With Quote
 
Tim Rentsch
Guest
Posts: n/a
 
      06-15-2013
Bart van Ingen Schenau <(E-Mail Removed)> writes:

> On Wed, 12 Jun 2013 15:24:50 -0700, Tim Rentsch wrote:
>
>> Eric Sosman <(E-Mail Removed)> writes:
>>
>>> Meanwhile, *I'm* puzzled by one feature of the code you
>>> show, and hope the experts will chime in. O Experts: Since
>>> lines 24, 26, 27, and 29 mention `struct pci_dev*' in the
>>> function prototypes, do these functions actually accept pointers
>>> to the type declared in line 33? Absent any earlier `struct
>>> pci_dev' declaration, aren't the others at "function prototype
>>> scope?"

>>
>> Yes, they are. Moreover, since they are, any attempt to
>> supply an argument to one of these functions (and assuming
>> there is no earlier declaration of struct pci_dev) will
>> result in a constraint violation (not counting the obvious
>> circumstances of giving a null pointer constant or an
>> expression of type (void *) as an argument). So in all
>> likelihood any TU that includes this file either has a
>> previous declaration of struct pci_dev or does not call
>> these functions.

>
> Or, as a third possibility, the compiler (settings) used to
> compile the software is non-conforming in that it does not
> diagnose the constraint violation.


No, there are only those two possibilities that avoid being a
constraint violation. Using non-conforming compiler settings
doesn't alter whether a given piece of code has a constraint
violation (and my earlier comments don't say anything about
diagnostics).

More to the point, the idea that compiler settings were used
which avoided issuing any diagnostics for such code is highly
unlikely. Linux is compiled using gcc (among others). Every
setting of compiler options I tried with gcc, including the least
conforming ones I could think of, all produced a diagnostic (or
more than one) in such cases. So unless whoever was doing the
builds for that code was ignoring some of the warnings (which
of course is also a possibility), the most likely circumstance
is that any TU having a #include "portdrv.h" also had an earlier
declaration of struct pci_dev, ie, fixing the problem with
function declarations that have a struct pci_dev * parameter.
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Who can explain this bug? mathog C Programming 57 06-11-2013 10:09 PM
This looks like a Perl bug George Mpouras Perl Misc 18 04-21-2013 11:56 PM
Really throwing this out there - does anyone have a copy of my oldDancer web browser? steven.miale@gmail.com Python 1 04-10-2013 03:32 PM
Can *common* struct-members of 2 different struct-types, that are thesame for the first common members, be accessed via pointer cast to either struct-type? John Reye C Programming 28 05-08-2012 12:24 AM
struct my_struct *p = (struct my_struct *)malloc(sizeof(struct my_struct)); Chris Fogelklou C Programming 36 04-20-2004 08:27 AM



Advertisments