Fix for spawning windows processes

Fridrich Strba fridrich.strba at bluewin.ch
Wed Apr 28 05:33:38 PDT 2010


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

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


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


More information about the dbus mailing list