Mailing List Archive

thanks - applied as e76069191

On Thu, 13 May 2021, Michael Forney wrote:

> This expands to a series of function definitions, so the semicolon is
> not necessary (in fact, it is not allowed in ISO C).
> ---
> krl.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/krl.c b/krl.c
> index 5612e774..c75279f9 100644
> --- a/krl.c
> +++ b/krl.c
> @@ -61,7 +61,7 @@ struct revoked_serial {
> };
> static int serial_cmp(struct revoked_serial *a, struct revoked_serial *b);
> RB_HEAD(revoked_serial_tree, revoked_serial);
> -RB_GENERATE_STATIC(revoked_serial_tree, revoked_serial, tree_entry, serial_cmp);
> +RB_GENERATE_STATIC(revoked_serial_tree, revoked_serial, tree_entry, serial_cmp)
>
> /* Tree of key IDs */
> struct revoked_key_id {
> @@ -70,7 +70,7 @@ struct revoked_key_id {
> };
> static int key_id_cmp(struct revoked_key_id *a, struct revoked_key_id *b);
> RB_HEAD(revoked_key_id_tree, revoked_key_id);
> -RB_GENERATE_STATIC(revoked_key_id_tree, revoked_key_id, tree_entry, key_id_cmp);
> +RB_GENERATE_STATIC(revoked_key_id_tree, revoked_key_id, tree_entry, key_id_cmp)
>
> /* Tree of blobs (used for keys and fingerprints) */
> struct revoked_blob {
> @@ -80,7 +80,7 @@ struct revoked_blob {
> };
> static int blob_cmp(struct revoked_blob *a, struct revoked_blob *b);
> RB_HEAD(revoked_blob_tree, revoked_blob);
> -RB_GENERATE_STATIC(revoked_blob_tree, revoked_blob, tree_entry, blob_cmp);
> +RB_GENERATE_STATIC(revoked_blob_tree, revoked_blob, tree_entry, blob_cmp)
>
> /* Tracks revoked certs for a single CA */
> struct revoked_certs {
> --
> 2.31.1
>
> _______________________________________________
> openssh-unix-dev mailing list
> openssh-unix-dev@mindrot.org
> https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
>
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

On Thu, 13 May 2021, Michael Forney wrote:
> This expands to a series of function definitions, so the semicolon is
> not necessary (in fact, it is not allowed in ISO C).

I went looking for that, and failed to find it.  The best I could find
says otherwise.  ISO/IEC 9899:2017 (C17): Section 6.8.3 Expression and
null statements specifically allows a null statement (as you'd expect
given the section name).

It must be a new revision.  When did the null statement become
disallowed?  Reference, please.

If the null statements are still allowed, I urge that the patch be
reverted as it would then be mere noise in the change history, a
distraction at best, and a source of errors at worst.

The benefit of the semi-colon (if allowed) is that it makes explicit
that the macro is a psuedo-statement.  Also, if the macro is redefined
to produce an expression that is not a (terminated) statement, the
program will no longer compile.

In case you hadn't noticed, I look unfavourably on trivial, janitorial
patches.

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

On 2021-06-04, David Newall <openssh@davidnewall.com> wrote:
> On Thu, 13 May 2021, Michael Forney wrote:
>> This expands to a series of function definitions, so the semicolon is
>> not necessary (in fact, it is not allowed in ISO C).
>
> I went looking for that, and failed to find it. The best I could find
> says otherwise. ISO/IEC 9899:2017 (C17): Section 6.8.3 Expression and
> null statements specifically allows a null statement (as you'd expect
> given the section name).
>
> It must be a new revision. When did the null statement become
> disallowed? Reference, please.

I'm not sure why statements are being discussed here. The patch is
about top-level external definitions (declarations and function
definitions), not statements. As you point out, null statements are
perfectly valid. Null declarations are not.

The relevant sections of the C standard are external definitions
(https://port70.net/~nsz/c/c99/n1256.html#6.9) and declarations
(https://port70.net/~nsz/c/c99/n1256.html#6.7).

I'll reproduce these parts of the grammar:

translation-unit:
external-declaration
translation-unit external-declaration
external-declaration:
function-definition
declaration

function-definition:
declaration-specifiers declarator declaration-listopt
compound-statement
compound-statement:
{ block-item-listopt }

declaration:
declaration-specifiers init-declarator-listopt ;
declaration-specifiers:
storage-class-specifier declaration-specifiersopt
type-specifier declaration-specifiersopt
type-qualifier declaration-specifiersopt
function-specifier declaration-specifiersopt

As you can see, a function definition ends with a '}', not an optional
semicolon. Additionally, there is no "null declaration",
declaration-specifiers are a required part of a declaration. This has
always been the case, from C89 to the latest C23 draft.

If you enable -Wpedantic, gcc will flag these null declarations:

$ echo ';' | gcc -Wpedantic -c -x c -
<stdin>:1:1: warning: ISO C does not allow extra ';' outside of a
function [-Wpedantic]

> If the null statements are still allowed, I urge that the patch be
> reverted as it would then be mere noise in the change history, a
> distraction at best, and a source of errors at worst.
>
> The benefit of the semi-colon (if allowed) is that it makes explicit
> that the macro is a psuedo-statement. Also, if the macro is redefined
> to produce an expression that is not a (terminated) statement, the
> program will no longer compile.

If the macro is redefined to produce an expression or statement, it
won't compile because statements and expressions aren't allowed at
top-level, only in function bodies. The best you could do if you
wanted to add a semicolon after a macro that expands to a function
definition would be to require C11 and use a dummy `_Static_assert(1,
"")` at the end.

If you care deeply about extending C with null declarations, I suggest
you send a proposal to WG14.
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

On 5/6/21 9:03 am, Michael Forney wrote:
> I'm not sure why statements are being discussed here. The patch is
> about top-level external definitions (declarations and function
> definitions), not statements. As you point out, null statements are
> perfectly valid. Null declarations are not.

I hadn't properly looked at your patch and misunderstood what you were
saying.

I apologise.

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev