From bb286988abb28f166a28fe2bf8e99a47508e414a 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 | 3 ++- 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, 29 insertions(+), 22 deletions(-) diff --git a/lib/commonio.c b/lib/commonio.c index ccc0cbd5f..b6c7b12d0 100644 --- a/lib/commonio.c +++ b/lib/commonio.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -642,7 +643,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 a43c2ac13..325702c2f 100644 --- a/lib/fputsx.c +++ b/lib/fputsx.c @@ -9,6 +9,7 @@ #include +#include #include #include @@ -25,7 +26,7 @@ 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 NULL; } else { diff --git a/lib/getdate.y b/lib/getdate.y index cc3d672fc..058600abb 100644 --- a/lib/getdate.y +++ b/lib/getdate.y @@ -25,6 +25,7 @@ #endif #include +#include #include #include #include @@ -934,7 +935,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 39bf054e7..559b4e0ee 100644 --- a/lib/gshadow.c +++ b/lib/gshadow.c @@ -171,9 +171,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 8d5d9c40e..80a671404 100644 --- a/lib/hushed.c +++ b/lib/hushed.c @@ -13,6 +13,7 @@ #ident "$Id$" #include +#include #include #include #include @@ -73,7 +74,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 = streq(buf, pw->pw_shell) || streq(buf, pw->pw_name); diff --git a/lib/loginprompt.c b/lib/loginprompt.c index e776cab70..1b59f3385 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" @@ -83,9 +84,8 @@ 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 9ff81e46a..fda57de89 100644 --- a/lib/ttytype.c +++ b/lib/ttytype.c @@ -11,6 +11,7 @@ #ident "$Id$" +#include #include #include @@ -46,7 +47,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 b2aaa83e3..6bfc1fc29 100644 --- a/lib/user_busy.c +++ b/lib/user_busy.c @@ -12,6 +12,7 @@ #ident "$Id: $" #include +#include #include #include #include @@ -125,7 +126,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 89c9a9d76..a24b0cc17 100644 --- a/src/login_nopam.c +++ b/src/login_nopam.c @@ -95,7 +95,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 ac5c1123f..fa98be464 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 @@ -361,7 +362,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, "="); @@ -601,7 +602,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) {