[Toybox] [PATCH] crond: Fix timing

enh enh at google.com
Mon Jan 13 11:46:24 PST 2025


On Mon, Jan 13, 2025 at 2:32 PM Rob Landley <rob at landley.net> wrote:
>
> On 1/12/25 20:09, Kana Steimle wrote:
> > The changes are working fine for me so far. But after trying more
> > things with crond, I found a few more issues I wanted to fix, all
> > related to environment variables. Another patch is attached.
> > - When setting a variable in the crontab file, the value would have a
> > newline at the end of it. That's because getline() returns the line
> > with a newline at the end, and the code didn't strip that newline out.
> > - The HOME variable was being sourced from what HOME was set to when
> > crond was run, rather than what the user's home is in the passwd file.
> > I believe the intention was to set HOME to what it is in the passwd
> > file, but allow the crontab to override it.
> > - I decided to also set LOGNAME to the username of the crontab's
> > owner, since the posix manpage for crontab mentions that as one of the
> > variables that should be set.
>
> Applied, but:
>
> 1) the vfork() child shares the parent's mappings, so modifications to
> the memory affect the parent process when done from the child. I believe
> this includes the environment space, and thus changes to the environment
> variables from the child are visible/persistent in the parent.
>
> 2) setenv() mallocs a new string and leaks the old one, if any, so doing
> that a lot in a long-lived server process gradually expands its memory
> footprint until reboot. (This is why lib/env.c exists.)
>
> 3) I _think_ getpwnam() returns the same static structure over and over,
> but am not sure? (The man page isn't clear.)

it's _allowed_ to, but isn't _required_ to...

https://pubs.opengroup.org/onlinepubs/9799919799/functions/getpwnam.html

bionic in particular prefers thread-local storage in basically all of
these cases (because apps are inherently multi-threaded, and you have
no idea what the system's threads are up to).

> But now that a user has showed up, I'll bump crond.c to the top of my
> cleanup list to promote for the release after the one I'm working on now...
>
> Thanks,
>
> Rob
> _______________________________________________
> Toybox mailing list
> Toybox at lists.landley.net
> http://lists.landley.net/listinfo.cgi/toybox-landley.net


More information about the Toybox mailing list