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