Skip to content

Conversation

alejandro-colomar
Copy link
Collaborator

@alejandro-colomar alejandro-colomar commented Sep 29, 2025

Suggested-by: @blm768

Queued after #1272.

@alejandro-colomar alejandro-colomar changed the title shadow thread-safe variants shadow *get*ent_r() thread-safe variant functions Sep 29, 2025
@alejandro-colomar alejandro-colomar force-pushed the shadow_r branch 10 times, most recently from c6713c5 to db2f315 Compare September 29, 2025 15:37
@alejandro-colomar alejandro-colomar force-pushed the shadow_r branch 3 times, most recently from 593c73f to 5124f13 Compare September 29, 2025 20:08

int e;
char *p, *end;
size_t n, lssize, size;

Check warning

Code scanning / CodeQL

Expression has no effect Warning

This expression has no effect.

struct group *sgetgrent(const char *s);
int sgetgrent_r(size_t size;
const char *restrict s, struct group *restrict grent,

Check notice

Code scanning / CodeQL

Short global name Note

Poor global variable name 's'. Prefer longer, descriptive names for globals (eg. kMyGlobalConstant, not foo).
static struct passwd pwent = {};

int e;
size_t size;

Check warning

Code scanning / CodeQL

Expression has no effect Warning

This expression has no effect.

struct passwd *sgetpwent(const char *s);
int sgetpwent_r(size_t size;
const char *restrict s, struct passwd *restrict pwent,

Check notice

Code scanning / CodeQL

Short global name Note

Poor global variable name 's'. Prefer longer, descriptive names for globals (eg. kMyGlobalConstant, not foo).
@alejandro-colomar alejandro-colomar force-pushed the shadow_r branch 5 times, most recently from e5f4e87 to ab412ac Compare September 30, 2025 07:39
sgetgrent(const char *s)
{
static char *buf = NULL;
static struct group grent = {};

Check notice

Code scanning / CodeQL

Local variable hides global variable Note

Local variable grent hides a
global variable
with the same name.
static struct subordinate_range sient = {};

int e;
size_t size;

Check warning

Code scanning / CodeQL

Expression has no effect Warning

This expression has no effect.
#ifdef ENABLE_SUBIDS
struct subordinate_range *sgetsient(const char *s);
int sgetsient_r(size_t size;
const char *restrict s, struct subordinate_range *restrict sient,

Check notice

Code scanning / CodeQL

Short global name Note

Poor global variable name 's'. Prefer longer, descriptive names for globals (eg. kMyGlobalConstant, not foo).
@alejandro-colomar alejandro-colomar changed the title shadow *get*ent_r() thread-safe variant functions shadow sgetXXent_r() thread-safe variant functions Sep 30, 2025
@alejandro-colomar alejandro-colomar force-pushed the shadow_r branch 7 times, most recently from 2d1b76a to 12b5f03 Compare October 2, 2025 10:42
This is for consistency with how this is done in sgetsgent().

Now we need to make sure that the list is NULL before the first call,
which means that the 'grent' structure must be cleared.  Since it's
a static object, it's already true, but let's be explicit about it.

Since we remember the address of the list in grent.gr_mem (which is
static), we don't need to keep 'members' being static too, so we get rid
of one static thing.

Signed-off-by: Alejandro Colomar <[email protected]>
Imitate how this is done in sgetgrent().

BE CAREFUL:  The NULL check needs to be performed after *both*
build_list() calls, since we need to make sure that either a NULL or a
valid pointer is stored in both .sg_adm and .sg_mem, since we'll call
free(3) next time sgetsgent() is called.

Signed-off-by: Alejandro Colomar <[email protected]>
Move build_list() to "lib/list.c", where we can reuse it.

Signed-off-by: Alejandro Colomar <[email protected]>
In some places we call comma_to_list(), and in others build_list(), and
they're essentially the same thing: they transform a comma-separated
list into an array of strings.

Both of them consider an empty list to have 0 members instead of one.
However, they differ in how they treat trailing commas:

-  comma_to_list() interprets that as an empty field.
-  build_list() ignores the trailing comma.

The behavior of build_list() seems more appropriate, since it allows
tools to generate the CSVs more easily.  Also, these lists are used to
represent user names or group names, so an empty user name makes no
sense.

This patch makes both functions be more consistent, which will
eventually allow us to de-duplicate code.

Signed-off-by: Alejandro Colomar <[email protected]>
get_gid() will fail to parse a number if the field is empty.

Signed-off-by: Alejandro Colomar <[email protected]>
This name tells better what this API does.

The name has a leading 'a', as it allocates memory, and then 'csv2ls'
because it parses a CSV into a list.

Signed-off-by: Alejandro Colomar <[email protected]>
We already use many GNU extensions, so it makes no sense to try to
conform to ISO C in this regard.

Signed-off-by: Alejandro Colomar <[email protected]>
The a2i() family of APIs (called a few lines later) already fails for an
empty string.  We don't need to explicitly reject it.

Signed-off-by: Alejandro Colomar <[email protected]>
Remove obvious comments, and improve (or make more consistent) the
useful ones.

Signed-off-by: Alejandro Colomar <[email protected]>
This will allow compacting the two buffers into one, which in turn, will
allow implementing a re-entrant variant of the function.

Signed-off-by: Alejandro Colomar <[email protected]>
This is in preparation for a following commit.

Signed-off-by: Alejandro Colomar <[email protected]>
This will allow compacting the two buffers into one, which in turn, will
allow implementing a re-entrant variant of the function.

Signed-off-by: Alejandro Colomar <[email protected]>
This makes this function consistent with the other sgetXXent()
functions.

Signed-off-by: Alejandro Colomar <[email protected]>
This will allow implementing a re-entrant variant of the function.

Signed-off-by: Alejandro Colomar <[email protected]>
This will reduce the diff when adding re-entrant variants of these
functions.

Signed-off-by: Alejandro Colomar <[email protected]>
This is not guaranteed to work by the C standard, but it's the only
thing we can do to have reasonable confidence that a pointer is aligned.

Signed-off-by: Alejandro Colomar <[email protected]>
Signed-off-by: Alejandro Colomar <[email protected]>
That change was temporary to allow adding a re-entrant variant of the
function with a small diff.  Remove it now that it's done.

Signed-off-by: Alejandro Colomar <[email protected]>
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.

1 participant