Differential D26734
crunchgen: replace "cd p->srcdir && pwd -P" with realpath(p->srcdir) Authored by danfe on Oct 11 2020, 8:23 AM. Tags None Referenced Files
Subscribers
Details rS366466 fixed a subtle bug by stripping the trailing \n appended to the output of the popen("cd %s && pwd -P", p->srcdir) call (pardon the pseudo-C syntax). Looks like the whole point of this call is to resolve the path to the p->srcdir which may contain symlinks. Consider replacing this logic with a single realpath(3) call which avoids spawning a shell, explicit reading from FILE *f with fgets(3), and strdup(3).
Diff Detail
Event TimelineComment Actions I suspect this is fine, but we'll need to look at existing usage to make sure some construct that we're currently expecting the shell to resolve can't sneak in here. Comment Actions This looks like the sane way of doing it. I think the only difference should be that the old version also gave an error if the path was not a directory, but I don't know if we need to check this here? Comment Actions Good point, we should is_dir() the result as well. If we don't, we will get down to the auto obj logic and could end up with a hard to debug mess depending on what we ended up with. Comment Actions Hmm, the p->srcdir upon which symlink expansion is performed comes from dir_search(p->name) call on line 650, and dir_search() does the is_dir() check internally itself. Could you confirm? Comment Actions Looks good to me, but maybe wait a little bit before committing so @kevans can have a look. |