Re: [PATCH 2/9] cmd: use GetCommandline() rather than argv

[Martin Wilck]

Hi Dan,

(re-cc'ing wine-devel)
> Running it through the buildbot now... seems ok on first two tries, so
> you may have accidentally fixed the problem.

I'll resubmit to wine-patches shortly, probably tonight.
> > > I ran the stuff through the bot, 0 errors everywhere except on NT (NT
> > > fails totally, do I really need to investigate that?).
> > > Yes, you may need to skip these tests on nt.
> > > I don't have a i7 machine,
> > > It's probably just mine that gets lucky.  Some tests
> > > fail better on some machines than others.
> > > Did you test with WINEDEBUG=warn+heap ?

No, I didn't. I thought you wree telling me that that made your problem
go away, so I didn't bother. I can do that of course if it helps.
> > but I tested on a Westmere EX server I have access to, no
> > problems with the conformance test. I also did some valgrind runs on
> > that machine, but I have little experience in reading valgrind output.
>
> Did you follow http://wiki.winehq.org/Wine_and_Valgrind ?
> Feel free to ask questions.

Yes, I did. I am seeing no regressions (read: additional warnings and/or
errors) between the trunk and my patched version.
> A few notes:
> +       if (!in_quotes && (*p == ' ' || *p == '\t')) {
>
> Could you use isspaceW() ?  I suppose not in the case statement below,
> but you check this in several places, and the function might be clearer
> (if it's correct)?

Honestly, I didn't consider it. I tried to change as little as possible
of the existing code.
> Is there duplication between WCMD_parameter and WCMD_parse
> that could be gotten rid of productively?

Yes, there is, as I noted in an early posting on this subject. Indeed
the two functions have essentially the same purpose. However I don't
feel bold enough to try fixing that as long as my current stuff isn't
accepted.

Originally, all that I wanted to fix was 'echo "Hi"'. I have gone quite
a few steps ahead from there with my current patch set. When I see this
getting merged I'll consider further fixes, but I won't be able to
follow up *much* further.

Best regards
Martin

[PATCH 2/9] cmd: use GetCommandline() rather than argv

[Dan Kegel]

On Sun, Sep 25, 2011 at 5:51 AM, Martin Wilck <mwilck(a)arcor.de> wrote:
> > Did you follow http://wiki.winehq.org/Wine_and_Valgrind ?
> > Feel free to ask questions.
>
> Yes, I did. I am seeing no regressions (read: additional warnings and/or
> errors) between the trunk and my patched version.

Great.

BTW I'd suggest sending in just the first patch or two of the
series this time.  That makes it sting less when you
have to resend.
- Dan