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>
"config.h" is a locally generated header. It must be included as
'#include "config.h"'.
It is already included correctly in some sources files. This commit
unifies the way how "config.h" is included.
Signed-off-by: Evgeny Grin (Karlson2k) <k2k@drgrin.dev>
We only call this function with a string literal, and it makes little
sense to pass something else. Let's simplify.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
with the meaning "a character was found".
strpbrk(3) is just like strchr(3), but searches for multiple characters.
Both functions have a boolean-like return value, which evaluates to true
if a character was found.
A better name for strpbrk(3) would have been strchrs().
Signed-off-by: Alejandro Colomar <alx@kernel.org>
This provides a safer and more consistent API.
We had the strrspn(3) function as it was for compatibility with Oracle
Solaris, but let's not repeat their mistake. Nevertheless, name our
function strrspn_() with a trailing underscore, to differentiate it from
the one in Solaris, since it's slightly different.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
This requires changing isspace(3) calls to an explicit accept string,
and I chose " \t\n" for it (as is done in other parts of this project),
which isn't exactly the same, but we probably don't want other
isspace(3) characters in those files, so it should work.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
e5905c4b ("Added control character check") introduced checking for
control characters but had the logic inverted, so it rejects all
characters that are not control ones.
Cast the character to `unsigned char` before passing to the character
checking functions to avoid UB.
Use strpbrk(3) for the illegal character test and return early.
* lib/fields.c (change_field): Don't point
before array start; that has undefined behavior.
Signed-off-by: Paul Eggert <eggert@cs.ucla.edu>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
* lib/fields.c (change_field): Since we know the string fits,
use strcpy(3) rather than strlcpy(3).
Signed-off-by: Paul Eggert <eggert@cs.ucla.edu>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
- Every non-const pointer converts automatically to void *.
- Every pointer converts automatically to void *.
- void * converts to any other pointer.
- const void * converts to any other const pointer.
- Integer variables convert to each other.
I changed the declaration of a few variables in order to allow removing
a cast.
However, I didn't attempt to edit casts inside comparisons, since they
are very delicate. I also kept casts in variadic functions, since they
are necessary, and in allocation functions, because I have other plans
for them.
I also changed a few casts to int that are better as ptrdiff_t.
This change has triggered some warnings about const correctness issues,
which have also been fixed in this patch (see for example src/login.c).
Signed-off-by: Alejandro Colomar <alx@kernel.org>
- Since strncpy(3) is not designed to write strings, but rather
(null-padded) character sequences (a.k.a. unterminated strings), we
had to manually append a '\0'. strlcpy(3) creates strings, so they
are always terminated. This removes dependencies between lines, and
also removes chances of accidents.
- Repurposing strncpy(3) to create strings requires calculating the
location of the terminating null byte, which involves a '-1'
calculation. This is a source of off-by-one bugs. The new code has
no '-1' calculations, so there's almost-zero chance of these bugs.
- strlcpy(3) doesn't padd with null bytes. Padding is relevant when
writing fixed-width buffers to binary files, when interfacing certain
APIs (I believe utmpx requires null padding at lease in some
systems), or when sending them to other processes or through the
network. This is not the case, so padding is effectively ignored.
- strlcpy(3) requires that the input string is really a string;
otherwise it crashes (SIGSEGV). Let's check if the input strings are
really strings:
- lib/fields.c:
- 'cp' was assigned from 'newft', and 'newft' comes from fgets(3).
- lib/gshadow.c:
- strlen(string) is calculated a few lines above.
- libmisc/console.c:
- 'cons' comes from getdef_str, which is a bit cryptic, but seems
to generate strings, I guess.1
- libmisc/date_to_str.c:
- It receives a string literal. :)
- libmisc/utmp.c:
- 'tname' comes from ttyname(3), which returns a string.
- src/su.c:
- 'tmp_name' has been passed to strcmp(3) a few lines above.
Signed-off-by: Alejandro Colomar <alx@kernel.org>
entry validity before commits to databases.
* libmisc/fields.c, libmisc/Makefile.am, lib/fields.c,
lib/Makefile.am, po/POTFILES.in: fields.c moved from libmisc to
lib.
