Re: [ros-dev] [ros-diffs] [hbelusca] 74528: Code simplifications.

[Thomas Faber]

On 2017-05-13 18:13, [e-mail] wrote:
> --- branches/setup_improvements/base/setup/usetup/filesup.c	[iso-8859-1] (original)
> +++ branches/setup_improvements/base/setup/usetup/filesup.c	[iso-8859-1] Sat May 13 16:13:49 2017
> @@ -524,14 +523,10 @@
>                          &IoStatusBlock,
>                          0,
>                          FILE_SYNCHRONOUS_IO_NONALERT);
>
> if (!NT_SUCCESS(Status))
>
> {
>  return FALSE;
>
> }
>
> NtClose(FileHandle);
>
> return TRUE;
> return NT_SUCCESS(Status);
>
> }

Not an improvement. You're now closing an uninitialized handle on
failure.

[Hermès BÉLUSCA-MAÏTO]

Fixed in r74538.

-----Message d'origine-----
De : Ros-dev [mailto:[e-mail]] De la part de Thomas Faber
Envoyé : samedi 13 mai 2017 18:26
À : [e-mail]
Objet : Re: [ros-dev] [ros-diffs] [hbelusca] 74528: Code simplifications.

On 2017-05-13 18:13, [e-mail] wrote:
> --- branches/setup_improvements/base/setup/usetup/filesup.c	[iso-8859-1] (original)
> +++ branches/setup_improvements/base/setup/usetup/filesup.c	[iso-8859-1] Sat May 13 16:13:49 2017
> @@ -524,14 +523,10 @@
>                          &IoStatusBlock,
>                          0,
>                          FILE_SYNCHRONOUS_IO_NONALERT);
>
> if (!NT_SUCCESS(Status))
>
> {
>  return FALSE;
>
> }
>
> NtClose(FileHandle);
>
> return TRUE;
> return NT_SUCCESS(Status);
>
> }

Not an improvement. You're now closing an uninitialized handle on failure.

_______________________________________________
Ros-dev mailing list
[e-mail]
http://www.reactos.org/mailman/listinfo/ros-dev

[Eric Kohl]

Hermès,

why are you making such changes at all????
I do not see any benefit in this change apart from the obvious bug you
introduced.

Your change removes our ability to add debug code to the failure and
success code paths without making addition changes. The compiler will
most likely generate the same code for the old and the new code.

Result: No gain at all!

Eric

Am 13.05.2017 18:26, schrieb Thomas Faber:
> On 2017-05-13 18:13, [e-mail] wrote:
> > --- branches/setup_improvements/base/setup/usetup/filesup.c   
> > [iso-8859-1] (original)
> > +++ branches/setup_improvements/base/setup/usetup/filesup.c   
> > [iso-8859-1] Sat May 13 16:13:49 2017
> > @@ -524,14 +523,10 @@
> >                          &IoStatusBlock,
> >                          0,
> >                          FILE_SYNCHRONOUS_IO_NONALERT);
> >
> > if (!NT_SUCCESS(Status))
> >
> > {
> >  return FALSE;
> >
> > }
> >
> > NtClose(FileHandle);
> >
> > return TRUE;
> > return NT_SUCCESS(Status);
> >
> > }
>
> Not an improvement. You're now closing an uninitialized handle on
> failure.
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev