Fix for spawning windows processes

[Fridrich Strba]

Did "git format-patch 04ddf2e3aafa2284e75a65555776c3d7ac29a846" afresh.

Cheers

Fridrich

On Thu, 2010-04-29 at 11:36 +0200, Ralf Habacker wrote: 
> Fridrich Strba schrieb:
> > Hello, Joerg,
> >
> > Nice to see you again :)
> >
> > Now, I really would then prefer something like:
> >
> > if (!strings || !string[0])
> > return NULL;
> >
> > At least for my TrainedMonkey eyes it is a bit more readable. But,
> > again, no religion, just preference.
> >
> > Attached is the new patch
> >   
> Applying: Pass the environment to CreateProcessA correctly and be more 
> defensive
> t:/svn-src/dbus-freedesktop-src-ssh/.git/rebase-apply/patch:14: trailing 
> whitespace.
> compose_string (char **strings, char separator)
> t:/svn-src/dbus-freedesktop-src-ssh/.git/rebase-apply/patch:24: trailing 
> whitespace.
>   if (!strings || !strings[0])
> t:/svn-src/dbus-freedesktop-src-ssh/.git/rebase-apply/patch:25: trailing 
> whitespace.
>     return 0;
> t:/svn-src/dbus-freedesktop-src-ssh/.git/rebase-apply/patch:26: trailing 
> whitespace.
>   for (i = 0; strings[i]; i++)
> t:/svn-src/dbus-freedesktop-src-ssh/.git/rebase-apply/patch:27: trailing 
> whitespace.
>     n += strlen (strings[i]) + 1;
> error: patch failed: dbus/dbus-spawn-win.c:471
> error: dbus/dbus-spawn-win.c: patch does not apply
> Patch failed at 0001 Pass the environment to CreateProcessA correctly 
> and be more defensive
> 
> still problems with this patch
> 
> Regards
>  Ralf
> 
> > Cheers
> >
> > Fridrich
> >
> > On Wed, 2010-04-28 at 14:01 +0200, Jörg Barfurth wrote: 
> >   
> >> Fridrich Strba schrieb:
> >>     
> >>> I did it intentionally, but left it there because I wanted it to raise
> >>> the question. If you have only teh argv[0] in your argv, why to leave
> >>> the space after the name there? 
> >>>       
> >> But you have i==0 only, if there was no string at all in argv.
> >>
> >> The same issue still exists in your new patch. Now i==0 signifies that 
> >> strings is not NULL, but points at an empty string sequence (i.e. 
> >> strings[0] == NULL). So p still points at the start of the buffer. 
> >> Without the 'if(i)' check you'll write outside (before) the buffer in 
> >> that case.
> >>
> >>     
> >>> The windows documentation is saying
> >>> "The system adds a terminating null character to the command-line string
> >>> to separate the file name from the arguments. This divides the original
> >>> string into two strings for internal processing."
> >>> So, was wondering whether one if cannot be saved :)
> >>>
> >>>       
> >>> P.S.: If there is a good reason to do the previous thing, I am not
> >>> having any religious opinion about that. Just was wondering :)
> >>>
> >>>       
> >> It was simply good defensive style. Now that you are making the method 
> >> more generic, so that you can rule out empty string lists even less, I 
> >> strongly suggest reintroducing it.
> >>
> >> Ciao
> >>
> >> - Jörg
> >>
> >>
> >>     
> >>> On Wed, 2010-04-28 at 10:44 +0200, Ralf Habacker wrote: 
> >>>       
> >>>> Fridrich Strba schrieb:
> >>>>         
> >>>>> Hello,
> >>>>>
> >>>>> This patch is fixing a bug in dbus/dbus-spawn-win.c
> >>>>> Maybe the two (build_commandline and build_env_string) could share some
> >>>>> code.
> >>>>>
> >>>>>   
> >>>>>           
> >>>> is the following fix  intended ?
> >>>>
> >>>> @@ -492,20 +492,46 @@ build_commandline (char **argv)
> >>>>        p += strlen (argv[i]);
> >>>>        *(p++) = ' ';
> >>>>      }
> >>>> -  if (i)
> >>>> +//  if (i)
> >>>>      p--;
> >>>>    *p = '\0';
> >>>>  
> >>>>    return buf;
> >>>>  }
> >>>>
> >>>> regards 
> >>>> Ralf 
> >>>>
> >>>>         
> >>     
> >
> >
> >   
> > ------------------------------------------------------------------------
> >
> > _______________________________________________
> > dbus mailing list
> > dbus at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dbus
> 
> _______________________________________________
> dbus mailing list
> dbus at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dbus

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Pass-the-environment-to-CreateProcessA-correctly-and.patch
Type: text/x-patch
Size: 2513 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/dbus/attachments/20100429/b804e765/attachment.bin>