Re: services: Remove an unused variable.

[Frédéric Delanoy]

On Fri, Aug 26, 2011 at 17:35, Francois Gouget  wrote:
> Totally independent from my other (futur) services patches.
>
> programs/services/services.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/programs/services/services.c b/programs/services/services.c
> index 35edbba..ff824bc 100644
> --- a/programs/services/services.c
> +++ b/programs/services/services.c
> @@ -289,12 +289,11 @@ static void scmdatabase_autostart_services(struct scmdatabase *db)
>     size = i;
>     for (i = 0; i < size; i++)
>     {
> - DWORD err;
>
> const WCHAR *argv[2];
>         service = services_list[i];
>         argv[0] = service->name;
>         argv[1] = NULL;
> - err = service_start(service, 1, argv);
> - service_start(service, 1, argv);
>
> /* FIXME: do something if the service failed to start */
>         release_service(service);
>     }

Wouldn't it be better to do something with err rather than muting (as
the comment suggests)?

services: Remove an unused variable.

[Francois Gouget]

On Fri, 26 Aug 2011, Frédéric Delanoy wrote:
[...]
> > - DWORD err;
> >
> > const WCHAR *argv[2];
> >         service = services_list[i];
> >         argv[0] = service->name;
> >         argv[1] = NULL;
> > - err = service_start(service, 1, argv);
> > - service_start(service, 1, argv);
> >
> > /* FIXME: do something if the service failed to start */
> >         release_service(service);
> >     }
>
> Wouldn't it be better to do something with err rather than muting (as
> the comment suggests)?

Maybe but what? The FIXME does not even say. In the meantime a FIXME 
comment is not a good enough reason to keep a compilation warning imho. 
And conversely fixing a compilation warning is not 'muting' a FIXME 
comment.

services: Remove an unused variable.

[Frédéric Delanoy]

2011/8/27 Francois Gouget :
> On Fri, 26 Aug 2011, Frédéric Delanoy wrote:
> [...]
> > > - DWORD err;
> > >
> > > const WCHAR *argv[2];
> > >         service = services_list[i];
> > >         argv[0] = service->name;
> > >         argv[1] = NULL;
> > > - err = service_start(service, 1, argv);
> > > - service_start(service, 1, argv);
> > >
> > > /* FIXME: do something if the service failed to start */
> > >         release_service(service);
> > >     }
> >
> > Wouldn't it be better to do something with err rather than muting (as
> > the comment suggests)?
>
> Maybe but what? The FIXME does not even say. In the meantime a FIXME
> comment is not a good enough reason to keep a compilation warning imho.
> And conversely fixing a compilation warning is not 'muting' a FIXME
> comment.

Well what the warning complains about is not technically fixed, just hidden.
I agree warnings are bad in general, but in this very case, you don't
solve the problem (absence of error checking) but simply hide the
symptom (warning).
(You could convert the commented fixme into a real one instead)

This is no different than changing

rc = myFun1(...)
rc = myFun2(...)
ok (rc...)

into

myFun1(...)
rc = myFun2(...)
ok (rc...)

which is generally frowned upon by AJ

Frédéric