Subject: Re: [PATCH 2/6] wrapper.h: move declarations for wrapper.c functions from git-compat-util.h

From: Junio C Hamano <[email-masked]>
Date: Tue, 16 May 2023 15:08:40 -0700

Calvin Wan <[email-masked]> writes:

> Since the functions in wrapper.c are widely used across the codebase,
> include it by default in git-compat-util.h. A future patch will remove
> now unnecessary inclusions of wrapper.h from other files.

The description taken alone implies the move in the other direction
(i.e. taking decls from wrapper.h and moving them to
git-compat-util.h), but that the patch actually does makes sense.
Slim compat-util.h by moving decls for wrapper functions that are
defined in wrapper.c to wrapper.h, but to avoid impacting the users
right away, include wrapper.h in the compat-util.h that these users
expect to find these decls in.

> Signed-off-by: Calvin Wan <[email-masked]>
> ---
>  git-compat-util.h | 113 +---------------------------------------------
>  wrapper.h         | 111 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 112 insertions(+), 112 deletions(-)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 51af0a53aa..9898fe9374 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -626,8 +626,7 @@ static inline int git_has_dir_sep(const char *path)
>  #include "compat/bswap.h"
>  
>  #include "wildmatch.h"
> -
> -struct strbuf;
> +#include "wrapper.h"
>  
>  /* General helper functions */
>  NORETURN void usage(const char *err);
> @@ -1047,36 +1046,6 @@ static inline int cast_size_t_to_int(size_t a)
>  # define xalloca(size)      (xmalloc(size))
>  # define xalloca_free(p)    (free(p))
>  #endif
> -char *xstrdup(const char *str);
> -void *xmalloc(size_t size);
> -void *xmallocz(size_t size);
> -void *xmallocz_gently(size_t size);
> -void *xmemdupz(const void *data, size_t len);
> -char *xstrndup(const char *str, size_t len);
> -void *xrealloc(void *ptr, size_t size);
> -void *xcalloc(size_t nmemb, size_t size);
> -void xsetenv(const char *name, const char *value, int overwrite);
> -void *xmmap(void *start, size_t length, int prot, int flags, int fd, off_t offset);
> -const char *mmap_os_err(void);
> -void *xmmap_gently(void *start, size_t length, int prot, int flags, int fd, off_t offset);
> -int xopen(const char *path, int flags, ...);
> -ssize_t xread(int fd, void *buf, size_t len);
> -ssize_t xwrite(int fd, const void *buf, size_t len);
> -ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
> -int xdup(int fd);
> -FILE *xfopen(const char *path, const char *mode);
> -FILE *xfdopen(int fd, const char *mode);
> -int xmkstemp(char *temp_filename);
> -int xmkstemp_mode(char *temp_filename, int mode);
> -char *xgetcwd(void);
> -FILE *fopen_for_writing(const char *path);
> -FILE *fopen_or_warn(const char *path, const char *mode);
> -
> -/*
> - * Like strncmp, but only return zero if s is NUL-terminated and exactly len
> - * characters long.  If it is not, consider it greater than t.
> - */
> -int xstrncmpz(const char *s, const char *t, size_t len);
>  
>  /*
>   * FREE_AND_NULL(ptr) is like free(ptr) followed by ptr = NULL. Note
> @@ -1178,15 +1147,10 @@ static inline size_t xsize_t(off_t len)
>  	return (size_t) len;
>  }
>  
> -__attribute__((format (printf, 3, 4)))
> -int xsnprintf(char *dst, size_t max, const char *fmt, ...);
> -
>  #ifndef HOST_NAME_MAX
>  #define HOST_NAME_MAX 256
>  #endif
>  
> -int xgethostname(char *buf, size_t len);
> -
>  /* in ctype.c, for kwset users */
>  extern const unsigned char tolower_trans_tbl[256];
>  
> @@ -1427,72 +1391,6 @@ void bug_fl(const char *file, int line, const char *fmt, ...);
>  #endif
>  #endif
>  
> -enum fsync_action {
> -	FSYNC_WRITEOUT_ONLY,
> -	FSYNC_HARDWARE_FLUSH
> -};
> -
> -/*
> - * Issues an fsync against the specified file according to the specified mode.
> - *
> - * FSYNC_WRITEOUT_ONLY attempts to use interfaces available on some operating
> - * systems to flush the OS cache without issuing a flush command to the storage
> - * controller. If those interfaces are unavailable, the function fails with
> - * ENOSYS.
> - *
> - * FSYNC_HARDWARE_FLUSH does an OS writeout and hardware flush to ensure that
> - * changes are durable. It is not expected to fail.
> - */
> -int git_fsync(int fd, enum fsync_action action);
> -
> -/*
> - * Writes out trace statistics for fsync using the trace2 API.
> - */
> -void trace_git_fsync_stats(void);
> -
> -/*
> - * Preserves errno, prints a message, but gives no warning for ENOENT.
> - * Returns 0 on success, which includes trying to unlink an object that does
> - * not exist.
> - */
> -int unlink_or_warn(const char *path);
> - /*
> -  * Tries to unlink file.  Returns 0 if unlink succeeded
> -  * or the file already didn't exist.  Returns -1 and
> -  * appends a message to err suitable for
> -  * 'error("%s", err->buf)' on error.
> -  */
> -int unlink_or_msg(const char *file, struct strbuf *err);
> -/*
> - * Preserves errno, prints a message, but gives no warning for ENOENT.
> - * Returns 0 on success, which includes trying to remove a directory that does
> - * not exist.
> - */
> -int rmdir_or_warn(const char *path);
> -/*
> - * Calls the correct function out of {unlink,rmdir}_or_warn based on
> - * the supplied file mode.
> - */
> -int remove_or_warn(unsigned int mode, const char *path);
> -
> -/*
> - * Call access(2), but warn for any error except "missing file"
> - * (ENOENT or ENOTDIR).
> - */
> -#define ACCESS_EACCES_OK (1U << 0)
> -int access_or_warn(const char *path, int mode, unsigned flag);
> -int access_or_die(const char *path, int mode, unsigned flag);
> -
> -/* Warn on an inaccessible file if errno indicates this is an error */
> -int warn_on_fopen_errors(const char *path);
> -
> -/*
> - * Open with O_NOFOLLOW, or equivalent. Note that the fallback equivalent
> - * may be racy. Do not use this as protection against an attacker who can
> - * simultaneously create paths.
> - */
> -int open_nofollow(const char *path, int flags);
> -
>  #ifndef SHELL_PATH
>  # define SHELL_PATH "/bin/sh"
>  #endif
> @@ -1632,13 +1530,4 @@ static inline void *container_of_or_null_offset(void *ptr, size_t offset)
>  	((uintptr_t)&(ptr)->member - (uintptr_t)(ptr))
>  #endif /* !__GNUC__ */
>  
> -void sleep_millisec(int millisec);
> -
> -/*
> - * Generate len bytes from the system cryptographically secure PRNG.
> - * Returns 0 on success and -1 on error, setting errno.  The inability to
> - * satisfy the full request is an error.
> - */
> -int csprng_bytes(void *buf, size_t len);
> -
>  #endif
> diff --git a/wrapper.h b/wrapper.h
> index f0c7d0616d..c85b1328d1 100644
> --- a/wrapper.h
> +++ b/wrapper.h
> @@ -1,6 +1,42 @@
>  #ifndef WRAPPER_H
>  #define WRAPPER_H
>  
> +char *xstrdup(const char *str);
> +void *xmalloc(size_t size);
> +void *xmallocz(size_t size);
> +void *xmallocz_gently(size_t size);
> +void *xmemdupz(const void *data, size_t len);
> +char *xstrndup(const char *str, size_t len);
> +void *xrealloc(void *ptr, size_t size);
> +void *xcalloc(size_t nmemb, size_t size);
> +void xsetenv(const char *name, const char *value, int overwrite);
> +void *xmmap(void *start, size_t length, int prot, int flags, int fd, off_t offset);
> +const char *mmap_os_err(void);
> +void *xmmap_gently(void *start, size_t length, int prot, int flags, int fd, off_t offset);
> +int xopen(const char *path, int flags, ...);
> +ssize_t xread(int fd, void *buf, size_t len);
> +ssize_t xwrite(int fd, const void *buf, size_t len);
> +ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
> +int xdup(int fd);
> +FILE *xfopen(const char *path, const char *mode);
> +FILE *xfdopen(int fd, const char *mode);
> +int xmkstemp(char *temp_filename);
> +int xmkstemp_mode(char *temp_filename, int mode);
> +char *xgetcwd(void);
> +FILE *fopen_for_writing(const char *path);
> +FILE *fopen_or_warn(const char *path, const char *mode);
> +
> +/*
> + * Like strncmp, but only return zero if s is NUL-terminated and exactly len
> + * characters long.  If it is not, consider it greater than t.
> + */
> +int xstrncmpz(const char *s, const char *t, size_t len);
> +
> +__attribute__((format (printf, 3, 4)))
> +int xsnprintf(char *dst, size_t max, const char *fmt, ...);
> +
> +int xgethostname(char *buf, size_t len);
> +
>  /* set default permissions by passing mode arguments to open(2) */
>  int git_mkstemps_mode(char *pattern, int suffix_len, int mode);
>  int git_mkstemp_mode(char *pattern, int mode);
> @@ -33,4 +69,79 @@ void write_file(const char *path, const char *fmt, ...);
>  /* Return 1 if the file is empty or does not exists, 0 otherwise. */
>  int is_empty_or_missing_file(const char *filename);
>  
> +enum fsync_action {
> +	FSYNC_WRITEOUT_ONLY,
> +	FSYNC_HARDWARE_FLUSH
> +};
> +
> +/*
> + * Issues an fsync against the specified file according to the specified mode.
> + *
> + * FSYNC_WRITEOUT_ONLY attempts to use interfaces available on some operating
> + * systems to flush the OS cache without issuing a flush command to the storage
> + * controller. If those interfaces are unavailable, the function fails with
> + * ENOSYS.
> + *
> + * FSYNC_HARDWARE_FLUSH does an OS writeout and hardware flush to ensure that
> + * changes are durable. It is not expected to fail.
> + */
> +int git_fsync(int fd, enum fsync_action action);
> +
> +/*
> + * Writes out trace statistics for fsync using the trace2 API.
> + */
> +void trace_git_fsync_stats(void);
> +
> +/*
> + * Preserves errno, prints a message, but gives no warning for ENOENT.
> + * Returns 0 on success, which includes trying to unlink an object that does
> + * not exist.
> + */
> +int unlink_or_warn(const char *path);
> + /*
> +  * Tries to unlink file.  Returns 0 if unlink succeeded
> +  * or the file already didn't exist.  Returns -1 and
> +  * appends a message to err suitable for
> +  * 'error("%s", err->buf)' on error.
> +  */
> +int unlink_or_msg(const char *file, struct strbuf *err);
> +/*
> + * Preserves errno, prints a message, but gives no warning for ENOENT.
> + * Returns 0 on success, which includes trying to remove a directory that does
> + * not exist.
> + */
> +int rmdir_or_warn(const char *path);
> +/*
> + * Calls the correct function out of {unlink,rmdir}_or_warn based on
> + * the supplied file mode.
> + */
> +int remove_or_warn(unsigned int mode, const char *path);
> +
> +/*
> + * Call access(2), but warn for any error except "missing file"
> + * (ENOENT or ENOTDIR).
> + */
> +#define ACCESS_EACCES_OK (1U << 0)
> +int access_or_warn(const char *path, int mode, unsigned flag);
> +int access_or_die(const char *path, int mode, unsigned flag);
> +
> +/* Warn on an inaccessible file if errno indicates this is an error */
> +int warn_on_fopen_errors(const char *path);
> +
> +/*
> + * Open with O_NOFOLLOW, or equivalent. Note that the fallback equivalent
> + * may be racy. Do not use this as protection against an attacker who can
> + * simultaneously create paths.
> + */
> +int open_nofollow(const char *path, int flags);
> +
> +void sleep_millisec(int millisec);
> +
> +/*
> + * Generate len bytes from the system cryptographically secure PRNG.
> + * Returns 0 on success and -1 on error, setting errno.  The inability to
> + * satisfy the full request is an error.
> + */
> +int csprng_bytes(void *buf, size_t len);
> +
>  #endif /* WRAPPER_H */