Skip to content

Conversation

@alejandro-colomar
Copy link
Collaborator

@alejandro-colomar alejandro-colomar commented Nov 6, 2025

This is a subset of #1056 at v9e.

This PR improves style, in preparation for the following PR, which will remove fgetsx() and fputsx().


Revisions:

v2
  • Fix bogus removal. It looks like an issue from a previous rebase.
$ git rd 
1:  60efe6d94 = 1:  60efe6d94 lib/, src/: Consistently use sizeof() as if it were a function
2:  ccf0e096c ! 2:  374ab381d lib/, src/: Remove useless casts in fgets(3)
    @@ Commit message
     
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
    - ## lib/commonio.c ##
    -@@ lib/commonio.c: int commonio_open (struct commonio_db *db, int mode)
    - 
    -                   len = strlen (buf);
    -                   if (db->ops->cio_fgets(buf + len, buflen - len, db->fp) == NULL)
    --                          goto cleanup_buf;
    -           }
    -           stpsep(buf, "\n");
    - 
    -
      ## lib/setupenv.c ##
     @@ lib/setupenv.c: static void read_env_file (const char *filename)
        if (NULL == fp) {
3:  8572b8825 = 3:  4b4be6004 lib/, src/: Consistently use NULL with fgets(3)
v3
  • Rebase
$ git rd 
1:  60efe6d94 ! 1:  1070be67a lib/, src/: Consistently use sizeof() as if it were a function
    @@ lib/log.c: void dolastlog (
        if (NULL != ll) {
                *ll = newlog;
     @@ lib/log.c: void dolastlog (
    -   STRNCPY(newlog.ll_host, host);
    +   strncpy_a(newlog.ll_host, host);
      #endif
        if (   (lseek (fd, offset, SEEK_SET) != offset)
     -      || (write_full(fd, &newlog, sizeof newlog) == -1)) {
    @@ lib/loginprompt.c: login_prompt(char *name, int namesize)
     @@ lib/loginprompt.c: login_prompt(char *name, int namesize)
         */
      
    -   MEMZERO(buf);
    +   memzero_a(buf);
     -  if (fgets (buf, sizeof buf, stdin) != buf) {
     +  if (fgets(buf, sizeof(buf), stdin) != buf) {
                exit (EXIT_FAILURE);
    @@ lib/utmp.c: failtmp(const char *username, const struct utmpx *failent)
        }
      
     @@ lib/utmp.c: prepare_utmp(const char *name, const char *line, const char *host,
    -           STRNCPY(utent->ut_host, hostname);
    +           strncpy_a(utent->ut_host, hostname);
      #endif
      #if defined(HAVE_STRUCT_UTMPX_UT_SYSLEN)
     -          utent->ut_syslen = MIN (strlen (hostname),
    @@ src/chage.c
     @@ src/chage.c: static int new_fields (void)
        (void) puts ("");
      
    -   SNPRINTF(buf, "%ld", mindays);
    +   stprintf_a(buf, "%ld", mindays);
     -  change_field (buf, sizeof buf, _("Minimum Password Age"));
     +  change_field(buf, sizeof(buf), _("Minimum Password Age"));
        if (a2sl(&mindays, buf, NULL, 0, -1, LONG_MAX) == -1)
                return 0;
      
    -   SNPRINTF(buf, "%ld", maxdays);
    +   stprintf_a(buf, "%ld", maxdays);
     -  change_field (buf, sizeof buf, _("Maximum Password Age"));
     +  change_field(buf, sizeof(buf), _("Maximum Password Age"));
        if (a2sl(&maxdays, buf, NULL, 0, -1, LONG_MAX) == -1)
    @@ src/chage.c: static int new_fields (void)
      
     @@ src/chage.c: static int new_fields (void)
        else
    -           DAY_TO_STR(buf, lstchgdate);
    +           day_to_str_a(buf, lstchgdate);
      
     -  change_field (buf, sizeof buf, _("Last Password Change (YYYY-MM-DD)"));
     +  change_field(buf, sizeof(buf), _("Last Password Change (YYYY-MM-DD)"));
    @@ src/chage.c: static int new_fields (void)
     @@ src/chage.c: static int new_fields (void)
        }
      
    -   SNPRINTF(buf, "%ld", warndays);
    +   stprintf_a(buf, "%ld", warndays);
     -  change_field (buf, sizeof buf, _("Password Expiration Warning"));
     +  change_field(buf, sizeof(buf), _("Password Expiration Warning"));
        if (a2sl(&warndays, buf, NULL, 0, -1, LONG_MAX) == -1)
                return 0;
      
    -   SNPRINTF(buf, "%ld", inactdays);
    +   stprintf_a(buf, "%ld", inactdays);
     -  change_field (buf, sizeof buf, _("Password Inactive"));
     +  change_field(buf, sizeof(buf), _("Password Inactive"));
        if (a2sl(&inactdays, buf, NULL, 0, -1, LONG_MAX) == -1)
    @@ src/chage.c: static int new_fields (void)
      
     @@ src/chage.c: static int new_fields (void)
        else
    -           DAY_TO_STR(buf, expdate);
    +           day_to_str_a(buf, expdate);
      
     -  change_field (buf, sizeof buf,
     +  change_field(buf, sizeof(buf),
    @@ src/login.c: int main (int argc, char **argv)
                /* Make the login prompt look like we want it */
     -          if (gethostname (hostn, sizeof (hostn)) == 0) {
     +          if (gethostname(hostn, sizeof(hostn)) == 0) {
    -                   SNPRINTF(loginprompt, _("%s login: "), hostn);
    +                   stprintf_a(loginprompt, _("%s login: "), hostn);
                } else {
    -                   STRTCPY(loginprompt, _("login: "));
    +                   strtcpy_a(loginprompt, _("login: "));
     @@ src/login.c: int main (int argc, char **argv)
      #ifdef HAVE_LL_HOST               /* __linux__ || SUN4 */
    -                   if (!STRNEQ(ll.ll_host, "")) {
    +                   if (!strneq_a(ll.ll_host, "")) {
                                printf (_(" from %.*s"),
     -                                  (int) sizeof ll.ll_host, ll.ll_host);
     +                                  (int) sizeof(ll.ll_host), ll.ll_host);
2:  374ab381d = 2:  b24f30953 lib/, src/: Remove useless casts in fgets(3)
3:  4b4be6004 ! 3:  712490882 lib/, src/: Consistently use NULL with fgets(3)
    @@ lib/loginprompt.c
     @@ lib/loginprompt.c: login_prompt(char *name, int namesize)
         */
      
    -   MEMZERO(buf);
    +   memzero_a(buf);
     -  if (fgets(buf, sizeof(buf), stdin) != buf) {
     +  if (fgets(buf, sizeof(buf), stdin) == NULL)
                exit (EXIT_FAILURE);

@hallyn
Copy link
Member

hallyn commented Nov 28, 2025

LGTM - please ping me when you've rebased

sizeof(foo)

-  No spaces.
	Not:  sizeof (foo)
-  Parentheses.
	Not:  sizeof foo
-  No parentheses wrapping sizeof itself
	Not:  (sizeof foo)

This patch can be approximated with the following semantic patch:

	$ cat ~/tmp/spatch/sizeof.sp
	@@
	identifier a, b;
	@@

	- sizeof a->b
	+ sizeof(a->b)

	@@
	identifier a, b;
	@@

	- sizeof a.b
	+ sizeof(a.b)

	@@
	identifier x;
	@@

	- sizeof x
	+ sizeof(x)

	@@
	identifier x;
	@@

	- sizeof *x
	+ sizeof(*x)

	@@
	identifier x;
	@@

	- (sizeof(x))
	+ sizeof(x)

	@@
	identifier x;
	@@

	- (sizeof(*x))
	+ sizeof(*x)

Applied as

	$ find contrib/ lib* src/ -type f \
	| xargs spatch --sp-file ~/tmp/spatch/sizeof.sp --in-place;

The differences between the actual patch and the approximation via the
semantic patch from above are whitespace only.  When reviewing, it'll
be useful to diff with '-w'.

Link: <https://lkml.org/lkml/2012/7/11/103>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
This patch can be replicated with the following semantic patch:

	$ cat fgets_cast.sp
	@@
	expression a, b, c;
	@@

	- fgets(a, (int) (b), c)
	+ fgets(a, b, c)

	@@
	expression a, b, c;
	@@

	- fgets(a, (int) b, c)
	+ fgets(a, b, c)

	@@
	expression a, b, c;
	@@

	- fgetsx(a, (int) (b), c)
	+ fgetsx(a, b, c)

	@@
	expression a, b, c;
	@@

	- fgetsx(a, (int) b, c)
	+ fgetsx(a, b, c)

	@@
	expression a, b, c, p;
	@@

	- p->cio_fgets(a, (int) (b), c)
	+ p->cio_fgets(a, b, c)

	@@
	expression a, b, c, p;
	@@

	- p->cio_fgets(a, (int) b, c)
	+ p->cio_fgets(a, b, c)

which is applied as:

	$ find lib* src/ -type f \
	| xargs spatch --sp-file ~/tmp/spatch/fgets_cast.sp --in-place;

Signed-off-by: Alejandro Colomar <alx@kernel.org>
fgets(3) returns either NULL or the input pointer.  Checking for NULL is
more explicit, and simpler.

<stddef.h> 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->cio_fgets(a, b, c) == a
	+ p->cio_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->cio_fgets(a, b, c) != a
	+ p->cio_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 <alx@kernel.org>
@alejandro-colomar
Copy link
Collaborator Author

LGTM - please ping me when you've rebased

Thanks! Rebased.

@hallyn hallyn merged commit 1ebca41 into shadow-maint:master Nov 28, 2025
11 checks passed
@alejandro-colomar alejandro-colomar self-assigned this Dec 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants