Re: [PATCH] Helper function to shell quote all arg values at once.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Le lundi 21 août 2006 00:57, Junio C Hamano a écrit :
> Christian Couder <chriscool@xxxxxxxxxxxxx> writes:
> 
> > The new sq_quote_argv function is used to refactor the
> > tracing code in "git.c" and "exec_cmd.c".
> > This function allocates memory and fills it with a string
> > containing the quoted argument values. Then it returns a
> > pointer to this memory that must be freed afterwards.
> 
> Sorry, I do not see a point in this.
> 
> If your original were doing malloc-print-free per iteration,
> then perhaps it makes sense to first format all into one
> allocated buffer, print all, and then free at once, like this
> patch does.  But that was not what the original had.

I thought that malloc-print-free was Ok, since there is already sq_quote
in quote.c that does it and is used in connect.c (except perhaps that the result from sq_quote is not freed).

> If the new function were to get a (const char **) array and
> FILE *, and print them, quoted and separated with spaces, then
> it would have shortened what two call sites did, which would
> have been an improvement.  But that is not what this patch does,
> either.

The patch still shortens the 3 calls site in git.c (-8 lines) and exec_cmd.c (-5 lines).
But anyway I will rewrite it so that the new function takes (FILE*, const char **, int count) as argument.

Or perhaps I need only make this new function call sq_quote_argv ? So I only need to send another patch on top of this one ?

Thanks,
Christian.
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]