Re: appwiz.cpl: Removed sign comparison warning

[Nikolay Sivov]

On 8/20/2010 20:04, Marko Nikolic wrote:
> Changed variable type to match function return type.
>
> dlls/appwiz.cpl/appwiz.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/dlls/appwiz.cpl/appwiz.c b/dlls/appwiz.cpl/appwiz.c
> index ffd2b24..1b3370b 100644
> --- a/dlls/appwiz.cpl/appwiz.c
> +++ b/dlls/appwiz.cpl/appwiz.c
> @@ -406,7 +406,7 @@ static void UpdateButtons(HWND hWnd)
>   {
>       APPINFO *iter;
>       LVITEMW lvItem;
> - DWORD selitem = SendDlgItemMessageW(hWnd, IDL_PROGRAMS, LVM_GETNEXTITEM, -1,
> - LRESULT selitem = SendDlgItemMessageW(hWnd, IDL_PROGRAMS, LVM_GETNEXTITEM, -1,
>     LVNI_FOCUSED | LVNI_SELECTED);
>  BOOL enable_modify = FALSE;

There's no need for that, return value means integer item index. What 
are you fixing with that?

appwiz.cpl: Removed sign comparison warning

[Marko Nikolic]

Nikolay Sivov wrote:
> On 8/20/2010 20:04, Marko Nikolic wrote:
> > Changed variable type to match function return type.
> >
> > dlls/appwiz.cpl/appwiz.c |    2 +-
> >   1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/dlls/appwiz.cpl/appwiz.c b/dlls/appwiz.cpl/appwiz.c
> > index ffd2b24..1b3370b 100644
> > --- a/dlls/appwiz.cpl/appwiz.c
> > +++ b/dlls/appwiz.cpl/appwiz.c
> > @@ -406,7 +406,7 @@ static void UpdateButtons(HWND hWnd)
> >   {
> >       APPINFO *iter;
> >       LVITEMW lvItem;
> > - DWORD selitem = SendDlgItemMessageW(hWnd, IDL_PROGRAMS,
> >
> > LVM_GETNEXTITEM, -1,
> > - LRESULT selitem = SendDlgItemMessageW(hWnd, IDL_PROGRAMS,
> >
> > LVM_GETNEXTITEM, -1,
> >          LVNI_FOCUSED | LVNI_SELECTED);
> >       BOOL enable_modify = FALSE;
>
> There's no need for that, return value means integer item index. What
> are you fixing with that?

Hi Nikolay,

The above change suppresses sign comparison warning in the line

if (selitem != -1) ...

and two more places below. selitem is declared as unsigned (DWORD), so 
comparing with -1 produces warning. Since SendDlgItemMessageW anyway return 
LRESULT which is signed integer, patch changes the variable type to match 
function result and removes sign warnings.

BR,

Marko

appwiz.cpl: Removed sign comparison warning

[Nikolay Sivov]

On 8/21/2010 12:18, Marko Nikolic wrote:
> Nikolay Sivov wrote:
> > On 8/20/2010 20:04, Marko Nikolic wrote:
> > > Changed variable type to match function return type.
> > >
> > > dlls/appwiz.cpl/appwiz.c |    2 +-
> > >    1 files changed, 1 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/dlls/appwiz.cpl/appwiz.c b/dlls/appwiz.cpl/appwiz.c
> > > index ffd2b24..1b3370b 100644
> > > --- a/dlls/appwiz.cpl/appwiz.c
> > > +++ b/dlls/appwiz.cpl/appwiz.c
> > > @@ -406,7 +406,7 @@ static void UpdateButtons(HWND hWnd)
> > >    {
> > >        APPINFO *iter;
> > >        LVITEMW lvItem;
> > > - DWORD selitem = SendDlgItemMessageW(hWnd, IDL_PROGRAMS,
> > >
> > > LVM_GETNEXTITEM, -1,
> > > - LRESULT selitem = SendDlgItemMessageW(hWnd, IDL_PROGRAMS,
> > >
> > > LVM_GETNEXTITEM, -1,
> > >           LVNI_FOCUSED | LVNI_SELECTED);
> > >        BOOL enable_modify = FALSE;
> >
> > There's no need for that, return value means integer item index. What
> > are you fixing with that?
>
> Hi Nikolay,
>
> The above change suppresses sign comparison warning in the line
>  if (selitem != -1) ...
>
> and two more places below. selitem is declared as unsigned (DWORD), so
> comparing with -1 produces warning. Since SendDlgItemMessageW anyway return
> LRESULT which is signed integer, patch changes the variable type to match
> function result and removes sign warnings.

LRESULT is something that supposed to work anyway as I understand it, 
for this case I think it's
better to cast function return value to INT and use INT as a local type.
> BR,
>
> Marko

appwiz.cpl: Removed sign comparison warning

[Marko Nikolic]

Nikolay Sivov wrote:
> On 8/21/2010 12:18, Marko Nikolic wrote:
> > Nikolay Sivov wrote:
> > > On 8/20/2010 20:04, Marko Nikolic wrote:
> > > > Changed variable type to match function return type.
> > > >
> > > > dlls/appwiz.cpl/appwiz.c |    2 +-
> > > >    1 files changed, 1 insertions(+), 1 deletions(-)
> > > >
> > > > diff --git a/dlls/appwiz.cpl/appwiz.c b/dlls/appwiz.cpl/appwiz.c
> > > > index ffd2b24..1b3370b 100644
> > > > --- a/dlls/appwiz.cpl/appwiz.c
> > > > +++ b/dlls/appwiz.cpl/appwiz.c
> > > > @@ -406,7 +406,7 @@ static void UpdateButtons(HWND hWnd)
> > > >    {
> > > >        APPINFO *iter;
> > > >        LVITEMW lvItem;
> > > > - DWORD selitem = SendDlgItemMessageW(hWnd, IDL_PROGRAMS,
> > > >
> > > > LVM_GETNEXTITEM, -1,
> > > > - LRESULT selitem = SendDlgItemMessageW(hWnd, IDL_PROGRAMS,
> > > >
> > > > LVM_GETNEXTITEM, -1,
> > > >           LVNI_FOCUSED | LVNI_SELECTED);
> > > >        BOOL enable_modify = FALSE;
> > >
> > > There's no need for that, return value means integer item index. What
> > > are you fixing with that?
> >
> > Hi Nikolay,
> >
> > The above change suppresses sign comparison warning in the line
> >  if (selitem != -1) ...
> >
> > and two more places below. selitem is declared as unsigned (DWORD), so
> > comparing with -1 produces warning. Since SendDlgItemMessageW anyway
> > return LRESULT which is signed integer, patch changes the variable type
> > to match function result and removes sign warnings.
>
> LRESULT is something that supposed to work anyway as I understand it,
> for this case I think it's
> better to cast function return value to INT and use INT as a local type.

Hi Nikolay,

I don't think that casting is a good practice, it could just hide warnings 
instead of fixing them (ok, sometimes is necesary). However, in this case 
LRESULT (Long result) is anyway an int, 32 or 64 bit, depending on the 
platform.

As I can see it, using LRESULT or INT is the same. Or maybe I am missing 
something else...

Marko