This is the mail archive of the cygwin mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: cygstart patch


Dave, you're right!  I was forgetting the NUL in realloc.
I'm surprised that the original fix has been working for me.
What do you think about Chuck's concerns regarding strcat()
vs. strncat()?

So, after adding 1 to the realloc line, the patch follows
(I *didn't* regenerate this with diff, is that OK?
I just changed the "1" to "2" and removed some whitespace.):

--- ../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)+2)) == NULL) {
+                fprintf(stderr, "%s: memory allocation error\n", argv[0]);
+                exit(1);
+            }		
+            strcat(args, " ");
+            strcat(args, *rest);
         }
     }
 
@@ -359,7 +363,7 @@ int main(int argc, const char **argv)
     if (action)
         free(action);
     if (args)
-        free(workDir);
+        free(args);
     if (workDir)
         free(workDir);
     if (file)


> -----Original Message-----
> From: cygwin-owner@cygwin.com 
> [mailto:cygwin-owner@cygwin.com]On Behalf
> Of Dave Korn
> Sent: Thursday, March 03, 2005 8:59
> To: cygwin@cygwin.com
> Subject: RE: cygstart patch
> 
> 
> ----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/
> 
> 

--
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/


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]