Is this struct definition right?

[fl]

Hi,

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;


What do you think?

Thanks,

................
1 /*
2 * File: portdrv.h
3 * Purpose: PCI Express Port Bus Driver's Internal Data Structures
4 *
5 * Copyright (C) 2004 Intel
6 * Copyright (C) Tom Long Nguyen ([email protected])
7 */
8
9 #ifndef _PORTDRV_H_
10 #define _PORTDRV_H_
11
12 #include <linux/compiler.h>
13
14 #define PCIE_PORT_DEVICE_MAXSERVICES 4
15 /*
16 * According to the PCI Express Base Specification 2.0, the indices of
17 * the MSI-X table entires used by port services must not exceed 31
18 */
19 #define PCIE_PORT_MAX_MSIX_ENTRIES 32
20
21 #define get_descriptor_id(type, service) (((type - 4) << 4) | service)
22
23 extern struct bus_type pcie_port_bus_type;
24 extern int pcie_port_device_register(struct pci_dev *dev);
25 #ifdef CONFIG_PM
26 extern int pcie_port_device_suspend(struct device *dev);
27 extern int pcie_port_device_resume(struct device *dev);
28 #endif
29 extern void pcie_port_device_remove(struct pci_dev *dev);
30 extern int __must_check pcie_port_bus_register(void);
31 extern void pcie_port_bus_unregister(void);
32
33 struct pci_dev;

 

[Malcolm McLean]

Hi,



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.

 

[Philip Lantz]

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.

I'm curious what you mean by "if you try to redefine it the program will
break."

After the above declaration, you can do any or all of the following:

struct pci_dev;
struct pci_dev { int x; };
void z() { struct pci_dev { double y; }; }

So, what are you thinking of that "will break"?

 

[Malcolm McLean]

I'm curious what you mean by "if you try to redefine it the program will

break."



After the above declaration, you can do any or all of the following:



struct pci_dev;

struct pci_dev { int x; };

void z() { struct pci_dev { double y; }; }



So, what are you thinking of that "will break"?

I was wrong.
I thought it prevented a redeclaration of the same symbol, but it doesn't
provide any such protection.

 

[Eric Sosman]

Hi,

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;


What do you think?

Thanks,

...............
1 /*
2 * File: portdrv.h
3 * Purpose: PCI Express Port Bus Driver's Internal Data Structures
4 *
5 * Copyright (C) 2004 Intel
6 * Copyright (C) Tom Long Nguyen ([email protected])
7 */
8
9 #ifndef _PORTDRV_H_
10 #define _PORTDRV_H_
11
12 #include <linux/compiler.h>
13
14 #define PCIE_PORT_DEVICE_MAXSERVICES 4
15 /*
16 * According to the PCI Express Base Specification 2.0, the indices of
17 * the MSI-X table entires used by port services must not exceed 31
18 */
19 #define PCIE_PORT_MAX_MSIX_ENTRIES 32
20
21 #define get_descriptor_id(type, service) (((type - 4) << 4) | service)
22
23 extern struct bus_type pcie_port_bus_type;
24 extern int pcie_port_device_register(struct pci_dev *dev);
25 #ifdef CONFIG_PM
26 extern int pcie_port_device_suspend(struct device *dev);
27 extern int pcie_port_device_resume(struct device *dev);
28 #endif
29 extern void pcie_port_device_remove(struct pci_dev *dev);
30 extern int __must_check pcie_port_bus_register(void);
31 extern void pcie_port_bus_unregister(void);
32
33 struct pci_dev;

Line 33 declares the existence of a type `struct pci_dev', but
omits the details of its makeup; it is an "incomplete type." There
are essentially only two things you can do with an incomplete type:

- You can make pointer variables that point to it. You can't
point them at anything useful, because you can't create an
actual `struct pci_dev' object: For starters, the compiler
doesn't know how much memory it needs. But you can create
a `struct pci_dev *ptr;' variable, and if somebody magically
hands you a suitable value you can store it, copy it, move
it around, and hand it onward to other functions.

- You can complete it by writing another declaration, this one
with all the innards (the struct or union members, the actual
dimension of an array, et cetera).

So, what's the point? It's C's idiom for "opaque" or "abstract"
data types. Think of FILE, for example: <stdio.h> declares the FILE
type, declares functions like fopen() that return FILE* pointers, and
declares functions like fprintf() that accept them. You can call
fopen() and get back a FILE* pointer value, you can store it in a FILE*
variable, and you can pass the stored value to rewind() and putc() and
fclose() -- all without needing to worry about what the actual FILE
looks like. If somebody enhances the <stdio.h> machinery and needs
to change the internals of a FILE, you needn't worry: All that stuff
is internal to the C library, and you're happy to pass pointers around
without fretting over precisely what they point at.

(Disclaimer: FILE is not a perfect example. There's a quirk in
the wording of the C Standard that requires FILE *not* to be an
incomplete type -- but it's used just as if it were, by user-level
code at any rate. It's a familiar example of how one uses an
incomplete type, even though FILE is not actually incomplete.)

Back to the code that puzzles you: You've got an incomplete
type `struct pci_dev', and you've got several functions that take
`struct pci_dev*' pointers as arguments. Somewhere -- maybe later
in the same file, maybe in some other file -- there's probably a
function or two that "creates" or "allocates" or "reserves" a PCI
device and returns a `struct pci_dev*' pointer, in very much the
same way that fopen() returns a FILE*. Somewhere else there's
surely an actual fleshing-out of the `struct pci_dev', hidden away
in the innards of the PCI implementation where the PCI mavens can
muck with it and not disturb the callers who only care about the
pointers that get handed around.

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?"

 

[Keith Thompson]

I was wrong.
I thought it prevented a redeclaration of the same symbol, but it doesn't
provide any such protection.

It's an odd kind of "protection" you had in mind. The whole point of
being to declare an incomplete type like "struct pci_dev;" is that you
can complete the type later with a full definition.

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

 

[Keith Thompson]

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.

 

[Bart van Ingen Schenau]

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.

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.

Bart v Ingen Schenau

 

[Keith Thompson]

It's incorrect C; the "struct pci_dev" in the function prototype is
not the same as the one defined later.

In my experience, older compilers just accepted this, then compilers
gave warnings about it (which many programmers didn't understand), and
then programmers learned to put a "struct pci_dev" in front of such a
prototype, either to get rid of the warning, or because they figured
out why the warning was there and why the code was actually wrong. So
the OP either uses an old compiler, or is lax with his attitude
towards compiler warnings.

Not necessarily. If struct pci_dev is *also* declared prior to the
first reference to it in that file, then the "struct pci_dev;" near the
bottom of the file is just a redundant declaration, and all instances of
"struct pci_dev" refer to the same type.

Looking at the git history of that particular file, earlier versions
refer to "struct pci_dev" *without* declaring it in the file.

I've sent an e-mail to the authors of the change that introduced that
declaration, but I haven't heard back from them yet.

 

[Anand Hariharan]

[...]

     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/b...80c4804d4515d05c64/drivers/pci/pcie/portdrv.h

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).

- Anand

 

[Keith Thompson]

[...]

     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/b...80c4804d4515d05c64/drivers/pci/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.

 

[Tim Rentsch]

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.

 

[Tim Rentsch]

(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.

 

[Tim Rentsch]

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.

 

[Tim Rentsch]

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.

 

[Bart van Ingen Schenau]

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

 

[Tim Rentsch]

[...]

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/b...80c4804d4515d05c64/drivers/pci/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.

 

[Tim Rentsch]

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.