Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replace fgetsx() by fgets(3) and fputsx() by fputs(3) #1056

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
Draft

Replace fgetsx() by fgets(3) and fputsx() by fputs(3) #1056

wants to merge 10 commits into from

Commits on Nov 2, 2024

  1. lib/, src/: Use strsep(3) instead of strtok(3) 

    strsep(3) is stateless, and so is easier to reason about.

It also has a slight difference: strtok(3) jumps over empty fields,
while strsep(3) respects them as empty fields.  In most of the cases
where we were using strtok(3), it makes more sense to respect empty
fields, and this commit probably silently fixes a few bugs.

In other cases (most notably filesystem paths), contiguous delimiters
("//") should be collapsed, so strtok(3) still makes more sense there.
This commit doesn't replace such strtok(3) calls.

Signed-off-by: Alejandro Colomar <[email protected]>
    @alejandro-colomar
    alejandro-colomar committed Nov 2, 2024
    Configuration menu
    Copy the full SHA
    eb9e625 View commit details
    Browse the repository at this point in the history

  2. lib/, src/: Use str[n]cmp(3) instead of explicit byte comparisons 

    This is simpler to read, IMO.

Signed-off-by: Alejandro Colomar <[email protected]>
    @alejandro-colomar
    alejandro-colomar committed Nov 2, 2024
    Configuration menu
    Copy the full SHA
    5b14222 View commit details
    Browse the repository at this point in the history

  3. contrib/, lib/, src/: Use consistent style using strchr(3) in conditi… 

    …onals

For negative matches, use
	if (strchr(...) == NULL)

For positive matches, use the cast-to-bool operator:
	if (!!strchr(...))

For positive matches, when a variable is also set, use
	while (NULL != (p = strchr(...)))

Signed-off-by: Alejandro Colomar <[email protected]>
    @alejandro-colomar
    alejandro-colomar committed Nov 2, 2024
    Configuration menu
    Copy the full SHA
    20e903b View commit details
    Browse the repository at this point in the history

  4. lib/gshadow.c: endsgent(): Invert logic to reduce indentation 

    Signed-off-by: Alejandro Colomar <[email protected]>
    @alejandro-colomar
    alejandro-colomar committed Nov 2, 2024
    Configuration menu
    Copy the full SHA
    bbde782 View commit details
    Browse the repository at this point in the history

  5. contrib/, lib/, src/: Consistently use sizeof() as if it were a function 

    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 <[email protected]>
    @alejandro-colomar
    alejandro-colomar committed Nov 2, 2024
    Configuration menu
    Copy the full SHA
    e1d0706 View commit details
    Browse the repository at this point in the history

  6. lib/, src/: Remove useless casts in fgets(3) 

    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->fgets(a, (int) (b), c)
	+ p->fgets(a, b, c)

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

	- p->fgets(a, (int) b, c)
	+ p->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 <[email protected]>
    @alejandro-colomar
    alejandro-colomar committed Nov 2, 2024
    Configuration menu
    Copy the full SHA
    1cf6b14 View commit details
    Browse the repository at this point in the history

  7. 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.

<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->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 <[email protected]>
    @alejandro-colomar
    alejandro-colomar committed Nov 2, 2024
    Configuration menu
    Copy the full SHA
    3fc6e9e View commit details
    Browse the repository at this point in the history

  8. lib/, po/: Remove fgetsx() and fputsx() 

    It seems they never worked correctly.  Let's keep it simple and remove
support for escaped newlines.

Closes: <shadow-maint#1055>
Reported-by: Chris Hofstaedtler <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
    @alejandro-colomar
    alejandro-colomar committed Nov 2, 2024
    Configuration menu
    Copy the full SHA
    79adfd9 View commit details
    Browse the repository at this point in the history

  9. lib/: Use getline(3) instead of its pattern 

    Signed-off-by: Alejandro Colomar <[email protected]>
    @alejandro-colomar
    alejandro-colomar committed Nov 2, 2024
    Configuration menu
    Copy the full SHA
    5bac295 View commit details
    Browse the repository at this point in the history

  10. lib/gshadow.c: fgetsgent(): Don't use static variables 

    Reported-by: Chris Hofstaedtler <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
    @alejandro-colomar
    alejandro-colomar committed Nov 2, 2024
    Configuration menu
    Copy the full SHA
    b3acc48 View commit details
    Browse the repository at this point in the history