From 8929e11ed89940504e49890384a3cb6d3298df9d Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Sun, 21 Jul 2024 18:21:02 +0200 Subject: [PATCH] lib/, src/: Consistently use NULL with fgets(3) fgets(3) returns either NULL or the input pointer. Checking for NULL is more explicit, and simpler. is the header that provides NULL; add it where appropriate. The meat of this patch can be approximated with the following semantic patch: $ cat ~/tmp/spatch/fgets_null.sp @@ expression a, b, c; @@ - fgets(a, b, c) == a + fgets(a, b, c) != NULL @@ expression a, b, c; @@ - fgetsx(a, b, c) == a + fgetsx(a, b, c) != NULL @@ expression a, b, c, p; @@ - p->fgets(a, b, c) == a + p->fgets(a, b, c) != NULL @@ expression a, b, c; @@ - fgets(a, b, c) != a + fgets(a, b, c) == NULL @@ expression a, b, c; @@ - fgetsx(a, b, c) != a + fgetsx(a, b, c) == NULL @@ expression a, b, c, p; @@ - p->fgets(a, b, c) != a + p->fgets(a, b, c) == NUL Applied as $ find contrib/ lib* src/ -type f \ | xargs spatch --sp-file ~/tmp/spatch/fgets_null.sp --in-place; The differences between the actual patch and the approximation via the semantic patch from above are includes, whitespace, braces, and a case where there was an implicit pointer-to-bool comparison which I made explicit. When reviewing, it'll be useful to use git-diff(1) with '-w' and '--color-words=.'. Signed-off-by: Alejandro Colomar --- lib/commonio.c | 3 ++- lib/fields.c | 6 +++--- lib/fputsx.c | 5 +++-- lib/getdate.y | 3 ++- lib/gshadow.c | 3 +-- lib/hushed.c | 3 ++- lib/loginprompt.c | 6 +++--- lib/setupenv.c | 3 ++- lib/ttytype.c | 3 ++- lib/user_busy.c | 3 ++- src/login_nopam.c | 2 +- src/useradd.c | 13 +++++++------ 12 files changed, 30 insertions(+), 23 deletions(-) diff --git a/lib/commonio.c b/lib/commonio.c index 5a06a3e71..ef8512037 100644 --- a/lib/commonio.c +++ b/lib/commonio.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -641,7 +642,7 @@ int commonio_open (struct commonio_db *db, int mode) if (NULL == buf) goto cleanup_errno; - while (db->ops->fgets (buf, buflen, db->fp) == buf) { + while (db->ops->fgets(buf, buflen, db->fp) != NULL) { struct commonio_entry *p; while ( (strrchr (buf, '\n') == NULL) diff --git a/lib/fields.c b/lib/fields.c index 9c61b2209..c1a91d82b 100644 --- a/lib/fields.c +++ b/lib/fields.c @@ -12,8 +12,9 @@ #ident "$Id$" #include -#include +#include #include +#include #include "prototypes.h" #include "string/strchr/stpspn.h" @@ -79,9 +80,8 @@ change_field(char *buf, size_t maxsize, const char *prompt) printf ("\t%s [%s]: ", prompt, buf); (void) fflush (stdout); - if (fgets (newf, maxsize, stdin) != newf) { + if (fgets(newf, maxsize, stdin) == NULL) return; - } if (stpsep(newf, "\n") == NULL) return; diff --git a/lib/fputsx.c b/lib/fputsx.c index 72d620992..325702c2f 100644 --- a/lib/fputsx.c +++ b/lib/fputsx.c @@ -9,6 +9,7 @@ #include +#include #include #include @@ -25,9 +26,9 @@ fgetsx(/*@returned@*/char *restrict buf, int cnt, FILE *restrict f) char *ep; while (cnt > 0) { - if (fgets (cp, cnt, f) != cp) { + if (fgets(cp, cnt, f) == NULL) { if (cp == buf) { - return 0; + return NULL; } else { break; } diff --git a/lib/getdate.y b/lib/getdate.y index e269bd85b..744695e1b 100644 --- a/lib/getdate.y +++ b/lib/getdate.y @@ -25,6 +25,7 @@ #endif #include +#include #include #include #include @@ -933,7 +934,7 @@ main(void) (void) fflush (stdout); buff[MAX_BUFF_LEN] = 0; - while (fgets (buff, MAX_BUFF_LEN, stdin) && buff[0]) + while (fgets(buff, MAX_BUFF_LEN, stdin) != NULL && buff[0]) { d = get_date(buff, NULL); if (d == -1) diff --git a/lib/gshadow.c b/lib/gshadow.c index 3e9e5948d..eb0f3b82c 100644 --- a/lib/gshadow.c +++ b/lib/gshadow.c @@ -170,9 +170,8 @@ void endsgent (void) buflen *= 2; len = strlen (buf); - if (fgetsx(&buf[len], buflen - len, fp) != &buf[len]) { + if (fgetsx(&buf[len], buflen - len, fp) == NULL) return NULL; - } } stpsep(buf, "\n"); return (sgetsgent (buf)); diff --git a/lib/hushed.c b/lib/hushed.c index 817bd0845..9f5080c69 100644 --- a/lib/hushed.c +++ b/lib/hushed.c @@ -13,6 +13,7 @@ #ident "$Id$" #include +#include #include #include #include @@ -72,7 +73,7 @@ bool hushed (const char *username) if (NULL == fp) { return false; } - for (found = false; !found && (fgets(buf, sizeof(buf), fp) == buf);) { + for (found = false; !found && (fgets(buf, sizeof(buf), fp) != NULL);) { stpsep(buf, "\n"); found = (strcmp (buf, pw->pw_shell) == 0) || (strcmp (buf, pw->pw_name) == 0); diff --git a/lib/loginprompt.c b/lib/loginprompt.c index 2f43875a7..fc2580cee 100644 --- a/lib/loginprompt.c +++ b/lib/loginprompt.c @@ -12,8 +12,9 @@ #ident "$Id$" #include -#include #include +#include +#include #include "attr.h" #include "defines.h" @@ -82,9 +83,8 @@ void login_prompt (char *name, int namesize) */ MEMZERO(buf); - if (fgets(buf, sizeof(buf), stdin) != buf) { + if (fgets(buf, sizeof(buf), stdin) == NULL) exit (EXIT_FAILURE); - } if (stpsep(buf, "\n") == NULL) exit(EXIT_FAILURE); diff --git a/lib/setupenv.c b/lib/setupenv.c index 099112948..a95e573f5 100644 --- a/lib/setupenv.c +++ b/lib/setupenv.c @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -54,7 +55,7 @@ static void read_env_file (const char *filename) if (NULL == fp) { return; } - while (fgets(buf, sizeof(buf), fp) == buf) { + while (fgets(buf, sizeof(buf), fp) != NULL) { if (stpsep(buf, "\n") == NULL) break; diff --git a/lib/ttytype.c b/lib/ttytype.c index d10ae28fc..f480d1050 100644 --- a/lib/ttytype.c +++ b/lib/ttytype.c @@ -11,6 +11,7 @@ #ident "$Id$" +#include #include #include @@ -45,7 +46,7 @@ void ttytype (const char *line) perror (typefile); return; } - while (fgets(buf, sizeof(buf), fp) == buf) { + while (fgets(buf, sizeof(buf), fp) != NULL) { if (buf[0] == '#') { continue; } diff --git a/lib/user_busy.c b/lib/user_busy.c index 424d0d7e3..539382fc9 100644 --- a/lib/user_busy.c +++ b/lib/user_busy.c @@ -12,6 +12,7 @@ #ident "$Id: $" #include +#include #include #include #include @@ -124,7 +125,7 @@ static int check_status (const char *name, const char *sname, uid_t uid) if (NULL == sfile) { return 0; } - while (fgets(line, sizeof(line), sfile) == line) { + while (fgets(line, sizeof(line), sfile) != NULL) { if (strncmp (line, "Uid:\t", 5) == 0) { unsigned long ruid, euid, suid; diff --git a/src/login_nopam.c b/src/login_nopam.c index 0f3b235ae..1c31752d5 100644 --- a/src/login_nopam.c +++ b/src/login_nopam.c @@ -99,7 +99,7 @@ login_access(const char *user, const char *from) if (NULL != fp) { int lineno = 0; /* for diagnostics */ while ( !match - && (fgets(line, sizeof(line), fp) == line)) + && (fgets(line, sizeof(line), fp) != NULL)) { char *p; diff --git a/src/useradd.c b/src/useradd.c index 9e79afddb..983009546 100644 --- a/src/useradd.c +++ b/src/useradd.c @@ -18,16 +18,17 @@ #include #include #ifdef ENABLE_LASTLOG -#include +# include #endif /* ENABLE_LASTLOG */ #include #include #include #ifdef ACCT_TOOLS_SETUID -#ifdef USE_PAM -#include "pam_defs.h" -#endif /* USE_PAM */ +# ifdef USE_PAM +# include "pam_defs.h" +# endif /* USE_PAM */ #endif /* ACCT_TOOLS_SETUID */ +#include #include #include #include @@ -360,7 +361,7 @@ get_defaults(void) * Read the file a line at a time. Only the lines that have relevant * values are used, everything else can be ignored. */ - while (fgets(buf, sizeof(buf), fp) == buf) { + while (fgets(buf, sizeof(buf), fp) != NULL) { stpsep(buf, "\n"); cp = stpsep(buf, "="); @@ -600,7 +601,7 @@ set_defaults(void) goto skip; } - while (fgets(buf, sizeof(buf), ifp) == buf) { + while (fgets(buf, sizeof(buf), ifp) != NULL) { char *val; if (stpsep(buf, "\n") == NULL) {