crunchgen: replace "cd p->srcdir && pwd -P" with realpath(p->srcdir)

[danfe]

Summary

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).

[kevans]

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.

[arichardson]

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?

[kevans]

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.

[danfe]

@kevans wrote:

Good point, we should is_dir() the result as well.

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?

[danfe]

Uhm, ping?

[danfe]

@danfe wrote:

Uhm, ping?

Another tentative ping...

[danfe]

Rebase after recent changes. Guys, anything prevents this from going forward?

[arichardson]

Looks good to me, but maybe wait a little bit before committing so @kevans can have a look.

[danfe]

@arichardson wrote:

Looks good to me, but maybe wait a little bit before committing so @kevans can have a look.

So, Kyle, you've got anything to say?

[kevans]

Oh yeah, sorry. :-( Go for it!