On 4/19/23 15:19, Eric Blake wrote:
> On Wed, Apr 19, 2023 at 11:53:44AM +0100, Richard W.M. Jones wrote:
>> On Tue, Apr 18, 2023 at 07:26:21PM +0200, Laszlo Ersek wrote:
>>> (The changes in this patch are simple, but likely more controversial than
>>> the rest.)
>>>
>>> The following four components don't play nice together:
>>>
>>> - Needlessly spelling out "extern" for function declarations in header
>>> files. (C99 6.2.2p5: "If the declaration of an identifier for a function
>>> has no storage-class specifier, its linkage is determined exactly as if
>>> it were declared with the storage-class specifier extern [...]".)
>>
>> Well I didn't know that ...
>
> https://stackoverflow.com/questions/18171373/why-are-some-functions-declared-extern-and-header-file-not-included-in-source-in/18173554#18173554
>
> gives some more interesting details (including what happens if you
> declare a function more than once, using different specifiers across
> the declarations).
In 2009, I constructed the following table, and I've been using it since:
-----------------
Name space: ordinary identifiers
Declaration | Scope | Linkage
| Storage duration | Definition
------------------------+-------+--------------------------------------------------------------------+------------------------+--------------------------
int obj_1; | file | external ((6.1.2.2 p5)
| static (6.1.2.4 p2) | tentative (6.7.2 p2)
extern int obj_2; | file | same as any visible file scope declaration /
external (6.1.2.2 p4) | static (6.1.2.4 p2)* | none (6.7.2)
static int obj_3; | file | internal (6.1.2.2 p3)
| static (6.1.2.4 p2) | tentative (6.7.2 p2)
int obj_4 = 1; | file | external ((6.1.2.2 p5)
| static (6.1.2.4 p2) | external (6.7.2 p1)
extern int obj_5 = 1; | file | same as any visible file scope declaration /
external (6.1.2.2 p4) | static (6.1.2.4 p2)* | external (6.7.2 p1)
static int obj_6 = 1; | file | internal (6.1.2.2 p3)
| static (6.1.2.4 p2) | external (6.7.2 p1)
int fun_1(void); | file | same as any visible file scope declaration /
external (6.1.2.2 p5) | n/a (6.1.2.4) | none (6.5 p4 fn56, 6.7.1)
extern int fun_2(void); | file | same as any visible file scope declaration /
external (6.1.2.2 p4) | n/a (6.1.2.4) | none (6.5 p4 fn56, 6.7.1)
static int fun_3(void); | file | internal (6.1.2.2 p3)
| n/a (6.1.2.4) | none (6.5 p4 fn56, 6.7.1)
------------------------+-------+--------------------------------------------------------------------+------------------------+--------------------------
int obj_7; | block | none (6.1.2.2 p6)
| automatic (6.1.2.4 p3) | yes (6.1.2.4 p3, 6.5 p4)
extern int obj_8; | block | same as any visible file scope declaration /
external (6.1.2.2 p4) | static (6.1.2.4 p2)* | none (6.7.2)
static int obj_9; | block | none (6.1.2.2 p6)
| static (6.1.2.4 p2) | yes (6.1.2.4 p2, 6.5 p4)
int obj_10 = 1; | block | none (6.1.2.2 p6)
| automatic (6.1.2.4 p3) | yes (6.1.2.4 p3, 6.5 p4)
extern int obj_11 = 1; | block | INVALID (6.5.7 p4)**
| INVALID | INVALID
static int obj_12 = 1; | block | none (6.1.2.2 p6)
| static (6.1.2.4 p2) | yes (6.1.2.4 p2, 6.5 p4)
int fun_4(void); | block | same as any visible file scope declaration /
external (6.1.2.2 p5) | n/a (6.1.2.4) | none (6.5 p4 fn56, 6.7.1)
extern int fun_5(void); | block | same as any visible file scope declaration /
external (6.1.2.2 p4) | n/a (6.1.2.4) | none (6.5 p4 fn56, 6.7.1)
static int fun_6(void); | block | INVALID (6.1.2.2 p3 fn13, 6.5.1 p4)
| INVALID | INVALID
* any visible file scope declaration can only specify either external or
internal linkage
** such a declaration would specify "same as any visible file scope declaration
/ external (6.1.2.2 p4)" linkage, that is, external or internal (see *)
-----------------
All the references are to C89, unfortunately. I've never gotten around updating
them to C99. :/
>
> I'm okay with dropping extern on functions if you want, but that can
> be a separate patch; leaving them doesn't hurt either (other than the
> line length issues where your line splitting techniques are fine).
Removing the "extern"s is too much churn :) We use them very consistently, and
they are not "wrong" by any means. They're a bit uncomfortable in this case,
but if the current patch is tolerable, I'd prefer it over a global
de-extern-ing.
>
>>> +++ b/lib/internal.h
>>> @@ -407,7 +407,9 @@ extern int nbd_internal_wait_until_connected (struct
>>> nbd_handle *h)
>>> LIBNBD_ATTRIBUTE_NONNULL (1);
>>>
>>> /* crypto.c */
>>> -extern struct socket *nbd_internal_crypto_create_session (struct
>>> nbd_handle *, struct socket *oldsock)
>>> +extern struct socket *
>>> + nbd_internal_crypto_create_session (struct nbd_handle *,
>>> + struct socket *oldsock)
>>> LIBNBD_ATTRIBUTE_NONNULL (1, 2);
>
> Looks reasonable to me. emacs may have a bit of a difficulty in
> auto-indenting this back to the same place, but it is infrequent
> enough that manual override is still viable.
>
Thanks! I'll stick with this, then.
Laszlo
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
