Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm
Sender: cygwin-owner AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com
content-class: urn:content-classes:message
MIME-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Subject: RE: cygstart patch
Date: Thu, 3 Mar 2005 09:22:07 -0500
Message-ID: <49D88D820A7BC0479A7B0932D4219EFE1DD0EE@NAEAPAXREX04VA.nadsusea.nads.navy.mil>
From: "Derosa, Anthony  CIV NAVAIR 2035, 2, 205/214" <Anthony DOT Derosa1 AT navy DOT mil>
To: <cygwin AT cygwin DOT com>
Content-Transfer-Encoding: 8bit

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 AT cygwin DOT com 
> [mailto:cygwin-owner AT cygwin DOT com]On Behalf
> Of Dave Korn
> Sent: Thursday, March 03, 2005 8:59
> To: cygwin AT cygwin DOT 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/