On Monday 17 November 2014 22:58:26 Dan Fandrich wrote:
> On Mon, Nov 17, 2014 at 06:06:23PM +0100, Peter Wu wrote:
> > This is the second revision of the earlier patchset[1] which fixes tests
> > which broke when ran under LD_PRELOAD=libhostname.so with ASAN enabled
> > (cmake without CURLDEBUG). To facilitate that change, it was necessary
> > to change from a command string to a list of command arguments.
>
> Is this the only use case for this patch set? It's pretty intrusive,

Yes it is intrusive in the sense that it touches many lines.

> makes it more difficult to see the control flow,

How so? It is now more clear whether I/O redirection occurs or not.

> and can result in surprises when what seems like a normal shell command-line
> has subtle differences.

What kind of surprises? Treat it as a command string, not as your interactive
shell. I would be surprised if a combination of variables somehow resulted in
execution of a different command by using backticks for example.

> If this is the only case where this patch helps, I would rather see if there
> is another approach to solving it (e.g. linking sethostname.c directly into
> the test binary).

When compiling with debugging enabled, it is already possible to override the
host name. For the future it seems a better idea to explicitly format the
command as a list of program and options rather than a command string. That
would result in less surprises when the command contains meta-characters. The
shell is not needed anyway, perl can perfectly execute commands by itself.

Thank you for your comments!

-- 
Kind regards,
Peter
https://lekensteyn.nl
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html