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