[PATCH] Move implementation of <file_change_detection.h> into a C file

[Stefan Liebler]

Hi Florian,

I've run some builds on my s390x and x86_64 machines with your applied 
patch. I do not see the -Werror=maybe-uninitialized anymore.

Despite of the whitespace issues, this patch looks good to me.

At least the second line of arguments for __file_is_unchanged needs two 
more spaces (see below).

I've also opened include/file_change_detection.h and 
io/file_change_detection.c in my emacs and it showed more whitespace 
errors: tab vs whitespaces.
Please have a look at the attached patch to see what I mean. I'm not 
exactly sure if this is really required. If yes, please also check the 
remaining files in the patch.

On 2/17/20 12:28 PM, Florian Weimer wrote:
> file_change_detection_for_stat partially initialize
> struct file_change_detection in some cases, when the size member
> alone determines the outcome of all comparisons.  This results
> in maybe-uninitialized compiler warnings in case of sufficiently
> aggressive inlining.
> 
> Once the implementation is moved into a separate C file, this kind
> of inlining is no longer possible, so the compiler warnings are gone.
> 
> (Note that I could not reproduce this warning in my tests.)
> 
> -----
>   include/file_change_detection.h | 110 +++++++---------------------------
>   io/Makefile                     |   2 +-
>   io/Versions                     |   4 ++
>   io/file_change_detection.c      | 129 ++++++++++++++++++++++++++++++++++++++++
>   io/tst-file_change_detection.c  |  57 +++++++++---------
>   resolv/res_init.c               |   2 +-
>   resolv/resolv_conf.c            |   6 +-
>   7 files changed, 185 insertions(+), 125 deletions(-)
> 
> diff --git a/include/file_change_detection.h b/include/file_change_detection.h
> index aaed0a9b6d..f5220745a9 100644
> --- a/include/file_change_detection.h
> +++ b/include/file_change_detection.h
> @@ -16,9 +16,10 @@
>      License along with the GNU C Library; if not, see
>      <https://www.gnu.org/licenses/>.  */
>   
> -#include <errno.h>
> +#ifndef _FILE_CHANGE_DETECTION_H
> +#define _FILE_CHANGE_DETECTION_H
> +
>   #include <stdbool.h>
> -#include <stddef.h>
>   #include <stdio.h>
>   #include <sys/stat.h>
>   #include <sys/types.h>
> @@ -38,103 +39,32 @@ struct file_change_detection
>   
>   /* Returns true if *LEFT and *RIGHT describe the same version of the
>      same file.  */
> -static bool __attribute__ ((unused))
> -file_is_unchanged (const struct file_change_detection *left,
> -                   const struct file_change_detection *right)
> -{
> -  if (left->size < 0 || right->size < 0)
> -    /* Negative sizes are used as markers and never match.  */
> -    return false;
> -  else if (left->size == 0 && right->size == 0)
> -    /* Both files are empty or do not exist, so they have the same
> -       content, no matter what the other fields indicate.  */
> -    return true;
> -  else
> -    return left->size == right->size
> -      && left->ino == right->ino
> -      && left->mtime.tv_sec == right->mtime.tv_sec
> -      && left->mtime.tv_nsec == right->mtime.tv_nsec
> -      && left->ctime.tv_sec == right->ctime.tv_sec
> -      && left->ctime.tv_nsec == right->ctime.tv_nsec;
> -}
> +bool __file_is_unchanged (const struct file_change_detection *left,
> +                        const struct file_change_detection *right);
Here we need two more spaces before const.


Thanks,
Stefan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 20200218_file_change_detection_whitespace.patch
Type: text/x-patch
Size: 4133 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20200218/744adea6/attachment.bin>