Re: [PING] [PATCH v2] Resolve-flockfile-funlockfile-differences

[Florian Weimer <fweimer@redhat.com>]

* Cupertino Miranda via Libc-alpha:

> diff --git a/stdio-common/vfscanf-internal.c b/stdio-common/vfscanf-internal.c
> index 2ad34050f3..5b55db962b 100644
> --- a/stdio-common/vfscanf-internal.c
> +++ b/stdio-common/vfscanf-internal.c
> @@ -177,11 +177,15 @@
>           return EOF;                                                         \
>         }                                                                     \
>      } while (0)
> +
> +static inline void checked_IO_funlockfile(FILE *s);
> +static inline void checked_IO_flockfile(FILE *s);
> +
>  #define LOCK_STREAM(S)                                                       \
> -  __libc_cleanup_region_start (1, (void (*) (void *)) &_IO_funlockfile, (S)); \
> -  _IO_flockfile (S)
> +  __libc_cleanup_region_start (1, (void (*) (void *)) &checked_IO_funlockfile, (S)); \
> +  checked_IO_flockfile (S)
>  #define UNLOCK_STREAM(S)                                                     \
> -  _IO_funlockfile (S);                                                       \
> +  checked_IO_funlockfile (S);                                                \
>    __libc_cleanup_region_end (0)
>
>  struct ptrs_to_free
> @@ -197,6 +201,18 @@ struct char_buffer {
>    struct scratch_buffer scratch;
>  };
>
> +static inline void
> +checked_IO_funlockfile(FILE *s)
> +{
> +  _IO_funlockfile (s);
> +}
> +
> +static inline void
> +checked_IO_flockfile(FILE *s)
> +{
> +  _IO_flockfile (s);
> +}

A couple of suggestions:

We only need a wrapper for funlockfile.  We should avoid the forward
declaration.  I think we can keep calling _IO_funlockfile on the
no-cancel path.  The wrapper function should have the correct type, so
that the (technically undefined) cast in __libc_cleanup_region_start is
no longer needed.

Please file a bug, with a summary like ”fscanf may incorrectly unlock
FSETLOCKING_BYCALLER stream if canceled”.

Please let me know if you can make the adjustments.  I have verified
that the change fixes my test case.

Thanks,
Florian