RE: cygstart patch

["Dave Korn" <dave dot korn at artimi dot com>]

----Original Message----
>From: Charles Wilson
>Sent: 03 March 2005 06:59

> Derosa, Anthony CIV NAVAIR 2035, 2, 205/214 wrote:
>> I found a small bug and added a feature to the cygstart utility, which
>> is part of the cygutils package.  The feature that I added removes the
>> limit on the length of the command line arguments passed to the target
>> application, which was previously limited to MAX_PATH.  The bug I fixed
>> was in regard to freeing the variable "args" instead of tyring to free
>> "workDir" twice.  A patch and change log follow below.  As this is my
>> first contribution, please correct me if I did something incorrectly.   

> I'm not an expert on these issues; anybody else want to chime in here
> before I apply Anthony's patch?

>> --- ../cygutils-1.2.6/src/cygstart/cygstart.c	2002-03-16
>> 00:49:44.000000000 -0500 +++ src/cygstart/cygstart.c	2005-03-02
>> 09:16:00.383625000 -0500 @@ -340,14 +340,18 @@ int main(int argc, const
>> char **argv) 
>> 
>>      /* Retrieve any arguments */
>>      if (rest && *rest) {
>> -        if ((args = (char *) malloc(MAX_PATH+1)) == NULL) {
>> +        if ((args = (char *) malloc(strlen(*rest)+1)) == NULL) {
>>              fprintf(stderr, "%s: memory allocation error\n", argv[0]); 
>> exit(1); 
>> -        }
>> -        strncpy(args, *rest, MAX_PATH);
>> +        }
>> +        strcpy(args, *rest);
>>          while (rest++ && *rest) {
>> -            strncat(args, " ", MAX_PATH-strlen(args));
>> -            strncat(args, *rest, MAX_PATH-strlen(args));
>> +            if ( (args = (char *) realloc(args, strlen(args) 
>> + strlen(*rest) + 1)) == NULL) { 
>> +                fprintf(stderr, "%s: memory allocation error\n",
argv[0]); 
>> +                exit(1); 
>> +        } 
>> +            strcat(args, " ");
>> +            strcat(args, *rest);
>>          }
>>      }



  Let me see if I'm following this right:

  First you allocate enough space for the length of the 'rest' string (+
final NUL)
  Then you copy 'rest' into that space
  Then while there are any more args you realloc the space to extend it and
copy the new arg in place (along with a space).

  I think the realloc is off-by-one, isn't it?
  You already have args completely full with a string and NUL-term from the
initial malloc.
  You want to add another string and a space.
  So the final size is strlen of the original string + 1 for the space +
strlen of the string you are concatenating + 1 for the terminating NUL.
  But you only malloc the sum of the two strlens plus one.

  Have I misunderstood something?



    cheers,
       DaveK
-- 
Can't think of a witty .sigline today....


--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/