search

shadow-maint / shadow

Upstream shadow tree
Other
299 stars 230 forks source link

Dead code #1041

Open alejandro-colomar opened 3 months ago

alejandro-colomar commented 3 months ago

https://github.com/shadow-maint/shadow/blob/53ea42e67f6ed3ca1f7eae69a3992774be627a45/lib/valid.c#L41-L57

The only way to arrive at line 48 with

('\0' == ent->pw_passwd[0])

is that

(NULL == ent->pw_name)

But then, it means that the second part of the condition in line 53 is redundant, and can be removed.

hallyn commented 2 months ago

Hm, note it's (now, at least) only used in sulogin (single user login).

I think the second part of

if ((NULL == ent->pw_name) || ('\0' == ent->pw_passwd[0])) {

is actually the important part. The first part is there to guard the second check: If there was no entry, then ent->pw_passwd is NULL, so we can't check ent->pw_passwd[0].

It might be more intuitive if it was

if ((NULL == ent->pw_passwd) || ('\0' == ent->pw_passwd[0])) {

But I'm not sure if there is some corner case that would fail - perhaps pw_passwd would end up pointing at garbage from a previous call, while pw->pw_name is reliably NULLed.

alejandro-colomar commented 2 months ago

Yeah, I think something is obscure there, and I prefer to not touch it. I fear that we'll break it if we touch it. I think let's keep this issue open, and forget about it, until someone has a deep look at the logic of that program.