Re: [ccache] [PATCH v3] add support for '@' parameters

2012-07-30 Thread Boie, Andrew P 


 -Original Message-
 From: Jürgen Buchmüller [mailto:pullm...@t-online.de]
 Sent: Monday, July 30, 2012 4:46 PM
 To: Boie, Andrew P
 Cc: ccache@lists.samba.org
 Subject: Re: [ccache] [PATCH v3] add support for '@' parameters
 
 Am Montag, den 30.07.2012, 16:05 -0700 schrieb Andrew Boie:
 [..snip..]
  +   /* Trivial case; replace with 1 element */
  +   dest-argv[index] = x_strdup(src-argv[0]);
 [..snap..]
 
 Shouldn't dest-argv[index] be free()d before overwriting it?

This was already freed, look 7 lines up.

  +   /* Copy the new arguments into place */
  +   for (j = 0; j  src-argc; j++)
  +   dest-argv[j + index] = x_strdup(src-argv[j]);
 
 Shouldn't src-argv[j] be free()d after it is strdup()ed?
 Or alternatively: why not copy src-argv[j] an re-use it?

src array isn't touched by this function the way it is currently implemented; 
this is stated in the function comments.  I could change this to have the 
function consume src if desired; I'm not sure what other cases Joel had in mind 
for this function when he asked me to move it to common code.

Andrew

___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache

Re: [ccache] [PATCH v3] add support for '@' parameters

2012-07-30 Thread Jürgen Buchmüller 
Am Montag, den 30.07.2012, 16:05 -0700 schrieb Andrew Boie:
[..snip..]
 + /* Trivial case; replace with 1 element */
 + dest-argv[index] = x_strdup(src-argv[0]);
[..snap..]

Shouldn't dest-argv[index] be free()d before overwriting it?

 + /* Copy the new arguments into place */
 + for (j = 0; j  src-argc; j++)
 + dest-argv[j + index] = x_strdup(src-argv[j]);

Shouldn't src-argv[j] be free()d after it is strdup()ed?
Or alternatively: why not copy src-argv[j] an re-use it?

The code patch doesn't seem to be too clean.. just my 'feeling' :-P
Excuse me, if I overlooked something essential.

Juergen




___
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache