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

[Thomas Faber]

On 2017-05-13 18:13, hbelusca(a)svn.reactos.org 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
À : ros-dev(a)reactos.org
Objet : Re: [ros-dev] [ros-diffs] [hbelusca] 74528: Code simplifications.

On 2017-05-13 18:13, hbelusca(a)svn.reactos.org 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
Ros-dev(a)reactos.org
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, hbelusca(a)svn.reactos.org 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
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev