Re: pdh[3:3]: remove unnecessary tests done in PdhLookupPerfNameByIndexW

[Dmitry Timoshkov]

"Jeff Latimer" <lats(a)yless4u.com.au> wrote:
> --- a/dlls/pdh/pdh_main.c
> +++ b/dlls/pdh/pdh_main.c
> @@ -837,14 +837,7 @@ PDH_STATUS WINAPI PdhLookupPerfNameByIndexA( LPCSTR machine, DWORD index, LPSTR
>
> TRACE("%s %d %p %p\n", debugstr_a(machine), index, buffer, size);
>
> -    if (machine)
> -    {
> -        FIXME("remote machine not supported\n");
> -        return PDH_CSTATUS_NO_MACHINE;
> -    }
> -
>     if (!buffer || !size) return PDH_INVALID_ARGUMENT;
> -    if (!index) return ERROR_SUCCESS;

Changelog mentions PdhLookupPerfNameByIndexW by the change is in PdhLookupPerfNameByIndexA.
Why are you removing the checks in one place by leave the same in other one? Why the tests
are unnecessary?

-- 
Dmitry.

pdh[3:3]: remove unnecessary tests done in PdhLookupPerfNameByIndexW

[Jeff Latimer]

Dmitry Timoshkov wrote:
> "Jeff Latimer" <lats(a)yless4u.com.au> wrote:
> > --- a/dlls/pdh/pdh_main.c
> > +++ b/dlls/pdh/pdh_main.c
> > @@ -837,14 +837,7 @@ PDH_STATUS WINAPI PdhLookupPerfNameByIndexA( 
> > LPCSTR machine, DWORD index, LPSTR
> >
> > TRACE("%s %d %p %p\n", debugstr_a(machine), index, buffer, size);
> >
> > -    if (machine)
> > -    {
> > -        FIXME("remote machine not supported\n");
> > -        return PDH_CSTATUS_NO_MACHINE;
> > -    }
> > -
> >     if (!buffer || !size) return PDH_INVALID_ARGUMENT;
> > -    if (!index) return ERROR_SUCCESS;
>
> Changelog mentions PdhLookupPerfNameByIndexW by the change is in 
> PdhLookupPerfNameByIndexA.
> Why are you removing the checks in one place by leave the same in 
> other one? Why the tests
> are unnecessary?

The parameters, bufferW and sizeW passed by PdhLookupPerfNameByIndexA to 
PdhLookupPerfNameByIndexW are  local to PdhLookupPerfNameByIndexA.  The 
test is needed still in PdhLookupPerfNameByIndexA because the size and 
buffer are not passed but still need to be checked.

Jeff

pdh[3:3]: remove unnecessary tests done inPdhLookupPerfNameByIndexW

[Dmitry Timoshkov]

"Jeff Latimer" <lats(a)yless4u.com.au> wrote:
> > > --- a/dlls/pdh/pdh_main.c
> > > +++ b/dlls/pdh/pdh_main.c
> > > @@ -837,14 +837,7 @@ PDH_STATUS WINAPI PdhLookupPerfNameByIndexA( 
> > > LPCSTR machine, DWORD index, LPSTR
> > >
> > > TRACE("%s %d %p %p\n", debugstr_a(machine), index, buffer, size);
> > >
> > > -    if (machine)
> > > -    {
> > > -        FIXME("remote machine not supported\n");
> > > -        return PDH_CSTATUS_NO_MACHINE;
> > > -    }
> > > -
> > >     if (!buffer || !size) return PDH_INVALID_ARGUMENT;
> > > -    if (!index) return ERROR_SUCCESS;
> >
> > Changelog mentions PdhLookupPerfNameByIndexW by the change is in 
> > PdhLookupPerfNameByIndexA.
> > Why are you removing the checks in one place by leave the same in 
> > other one? Why the tests
> > are unnecessary?
>
> The parameters, bufferW and sizeW passed by PdhLookupPerfNameByIndexA to 
> PdhLookupPerfNameByIndexW are  local to PdhLookupPerfNameByIndexA.

That doesn't mean that PdhLookupPerfNameByIndexW can't be called directly.
> The 
> test is needed still in PdhLookupPerfNameByIndexA because the size and 
> buffer are not passed but still need to be checked.

That still doesn't answer my questions.

-- 
Dmitry.

pdh[3:3]: remove unnecessary tests done inPdhLookupPerfNameByIndexW

[Jeff Latimer]

Dmitry Timoshkov wrote:
> "Jeff Latimer" <lats(a)yless4u.com.au> wrote:
> > > > -    if (machine)
> > > > -    {
> > > > -        FIXME("remote machine not supported\n");
> > > > -        return PDH_CSTATUS_NO_MACHINE;
> > > > -    }
> > > > -
> > > >     if (!buffer || !size) return PDH_INVALID_ARGUMENT;
> > > > -    if (!index) return ERROR_SUCCESS;
> > > > Changelog mentions PdhLookupPerfNameByIndexW by the change is in 
> > > > PdhLookupPerfNameByIndexA.
> > > > Why are you removing the checks in one place by leave the same in 
> > > > other one? Why the tests
> > > > are unnecessary?
> > > > The parameters, bufferW and sizeW passed by PdhLookupPerfNameByIndexA 
> > > > to PdhLookupPerfNameByIndexW are  local to PdhLookupPerfNameByIndexA.
> > > > My understanding of your question is: if I am removing the tests in one 
> > > > place then they should not be be necessary in the other.  This is not 
> > > > the case.  A call to PdhLookupPerfNameByIndexA will call 
> > > > PdhLookupPerfNameByIndexW and thus execute the all tests.  The way it is 
> > > > at the moment the tests get executed twice (in the negative), this 
> > > > change ensures that they only execute once.
>
> That doesn't mean that PdhLookupPerfNameByIndexW can't be called 
> directly.
> Correct and that is why the tests are left in place.
> > The test is needed still in PdhLookupPerfNameByIndexA because the 
> > size and buffer are not passed but still need to be checked.
>
> That still doesn't answer my questions.
> Does this clarify this things?

pdh[3:3]: remove unnecessary tests doneinPdhLookupPerfNameByIndexW

[Dmitry Timoshkov]

"Jeff Latimer" <lats(a)yless4u.com.au> wrote:
> My understanding of your question is: if I am removing the tests in one 
> place then they should not be be necessary in the other.  This is not 
> the case.  A call to PdhLookupPerfNameByIndexA will call 
> PdhLookupPerfNameByIndexW and thus execute the all tests.  The way it is 
> at the moment the tests get executed twice (in the negative), this 
> change ensures that they only execute once.

That's true for index, but not for machine.

-- 
Dmitry.

pdh[3:3]: remove unnecessary tests doneinPdhLookupPerfNameByIndexW

[Jeff Latimer]

Dmitry Timoshkov wrote:
> That's true for index, but not for machine.
> You are correct. I will resubmit.  I think it will be better to pass 
> machine in this case.

Jeff

pdh[3:3]: remove unnecessary tests doneinPdhLookupPerfNameByIndexW

[Dmitry Timoshkov]

"Jeff Latimer" <lats(a)yless4u.com.au> wrote:
> I think it will be better to pass 
> machine in this case.

Yes, it just needs to be converted to unicode first.

-- 
Dmitry.