Hi Mike,
thanks for the reviews!
2016-03-17 18:23 GMT+01:00 Mike Frysinger <vap...@gentoo.org>:
> On 17 Mar 2016 15:52, Bartosz Golaszewski wrote:
>> +static void mount_procfs(const char *target)
>> +{
>> + int status;
>> +
>> + status = mount("none", target, NULL, MS_PRIVATE | MS_REC, NULL);
>> + if (status < 0)
>> + goto mount_err;
>> +
>> + status = mount("proc", target, "proc",
>> + MS_NOSUID | MS_NOEXEC | MS_NODEV, NULL);
>
> each of these mount calls could do with a comment explaining what/why.
> you & i might understand how /proc needs to be made private & then
> freshly mounted in a new pid ns, but not everyone :).
>
Done in v7.
>> + if (status < 0)
>> + goto mount_err;
>
> general style note ... seems like this could be written:
> status = mount(...)
> if (status == 0)
> status = mount(...)
> if (status < 0)
> bb_perror_msg_and_die(...)
>
> might be smaller code wise ?
>
Changed in v7.
>> + run_shell(getenv("SHELL"), 0, NULL, NULL);
>
> if SHELL isn't set, then we just segfault ?
Nope, run_shell() checks if const char *shell is NULL or an empty
string and runs DEFAULT_SHELL in that case.
Tested on my regular Debian system:
bash$ ./busybox unshare
bash$
bash$ SHELL="" ./busybox unshare
sh-4.3$
--
Best regards,
Bartosz Golaszewski
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
