[PATCH v3 04/27] adduser: replace BB_EXECLP call with BB_EXECVP

[Kang-Che Sung]

On Mon, Jan 27, 2025 at 4:13 PM <d+busybox at adaptive-enterprises.com> wrote:
>
> On Mon, 27 Jan 2025, Kang-Che Sung wrote:
> > On Mon, Jan 27, 2025 at 12:18 PM <d+busybox at adaptive-enterprises.com> wrote:
> >>> +     passwd_argv[0] = (char *) "passwd";
> >>> +     passwd_argv[1] = (char *) "--";
> >>> +     passwd_argv[2] = (char *) login_name;
> >>> +     passwd_argv[3] = NULL;
> >>
> >> The type of a string literal is already char *, so the first two don't need
> >> the cast. You could even write it more compactly, like this:
> >>
> >>         char *passwd_argv[] = { "passwd", "--", (char *)login_name, NULL };
> >
> > Technically, string literals are of type "const char *", not "char *".
>
> Perhaps in C++ that is true, but not C. Ref C23, s6.4.5 "String literals",
>
>      For character string literals, the array elements have type char, ...
>
> You can try a few compiler experiments if you like.

Perhaps I didn't explain clearly enough. There is a distinction
between when a string literal is specified in an array declaration and
when it's passed as a pointer:

char str[] = "passwd"; // The string has type "char[7]", and is
mutable (i.e. not placed in ".rodata" section)
char *ptr = "passwd"; // This string is placed in ".rodata" section,
and should be referred by the type "const char *".

> > Perhaps what you really intended is this?
> >
> > const char *passwd_argv[] = { "passwd", "--", login_name, NULL };
>
> That would be much more intuitive, readable, safer and so on, but then
> you'll need a cast when pass it to the pre-const functions,
> eg (char **)passwd_argv.

>From the surrounding code I've read, you are attempting to pass a
".rodata" string into an "argv" character pointer array.

But "argv" is supposed to be mutable by the callee (i.e. the new
program's main function). (I think it's required by the C standard.)

Unless there's a guarantee that the callee never modifies the "argv"
string, passing ".rodata" strings with de-const casts would be unsafe.
That's why when I initially saw the patch and code I was nervous. (A
(char *) de-const cast without an in-code explanation of why is a red
flag to me.)