Re: [ros-dev] [ros-diffs] [cgutman] 44286: - Initialize SocketError to 0 to prevent a bogus error from GCC

[Colin Finck]

cgutman(a)svn.reactos.org wrote:
>   -        ULONG SocketError;
>  +        ULONG SocketError = 0; 
FYI, I've already reported this bug some days ago at
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42145.
As it only occurs in GCC >= 4.3 and the necessary code changes for compiling
ReactOS with GCC 4.4.x are currently collected in a big separate patch (see
http://www.reactos.org/bugzilla/show_bug.cgi?id=4810), no hack from my side
has been committed so far.

If this hack is going to stay, please mark it as such and add a reference to
the GCC bug report.
Hopefully, it's properly resolved someday... :-)

Best regards,

Colin

[Dmitry Gorbachev]

Notice that the warning is "may be used uninitialized" and not "is
used uninitialized", so it is correct, in a sense.

[Alex Ionescu]

Microsoft deals with this often -- they even have a macro you should probably use:

//
// The following macro is used to satisfy the compiler. Note that the
// expressions/statements passed to this macro are not necessary for
// correctness, but without them the compiler cannot compile with W4.
//

#define SATISFY_OVERZEALOUS_COMPILER(X) X
You use it as such:

SATISFY_OVERZEALOUS_COMPILER (SocketError = 0);

On 2009-11-25, at 2:28 PM, Dmitry Gorbachev wrote:

>   Notice that the warning is "may be used
> uninitialized" and not "is
>  used uninitialized", so it is correct, in a sense.
>  
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev 
Best regards,
Alex Ionescu

[Thomas Bluemel]

What's the point of wrapping these initializations into macros?  Just 
initialize these variables to something meaningful.  If the compiler 
isn't sure (thus the warning), it will initialize the variable according 
to your code.  If it determines that that assignment is obsolete, the 
optimizer most likely removes it.  No ugly code neccessary.

Thomas
Alex Ionescu wrote:
>   Microsoft deals with this often -- they even have a
> macro you should 
>  probably use:
>
>  //
>  // The following macro is used to satisfy the compiler. Note that the
>  // expressions/statements passed to this macro are not necessary for
>  // correctness, but without them the compiler cannot compile with W4.
>  //
>
>  #define SATISFY_OVERZEALOUS_COMPILER(X) X
>  You use it as such:
>
>  SATISFY_OVERZEALOUS_COMPILER (SocketError = 0);
>    
>  On 2009-11-25, at 2:28 PM, Dmitry Gorbachev wrote:
>
> >   Notice that the warning is "may be used
> > uninitialized" and not "is
> >  used uninitialized", so it is correct, in a sense.
> >
> >  _______________________________________________
> >  Ros-dev mailing list
> >  Ros-dev(a)reactos.org <mailto:[e-mail]>
> >  http://www.reactos.org/mailman/listinfo/ros-dev 
>  Best regards,
>  Alex Ionescu
>
>  ------------------------------------------------------------------------
>
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev

[KJK::Hyperion]

Thomas Bluemel wrote:
>   What's the point of wrapping these initializations
> into macros? 
in Microsoft (and elsewhere in the "REAL WORLD") they have a thing
called a "CODE REVIEW"

[Thomas Bluemel]

If a compiler finds it too complex to determine whether a variable was 
initialized or not, you're probably better off initializing it 
unconditionally.  I don't quite see what this has to do with a code 
review apart from identifying such potentially buggy code (why write it 
in such a way in the first place?).  After fixing it there's no point in 
reviewing it over and over again, so there's really no reason for such 
an ugly macro.

Thomas
KJK::Hyperion wrote:
>   Thomas Bluemel wrote:
>    
> >   What's the point of wrapping these
> > initializations into macros?
> >       
>  in Microsoft (and elsewhere in the "REAL WORLD") they have a thing
>  called a "CODE REVIEW"
>
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev
>

[Dmitry Gorbachev]

Microsoft also develops a compiler. Maybe it is needed for it.

>   If a compiler finds it too complex to determine
> whether a variable was
>  initialized or not, you're probably better off initializing it
>  unconditionally.  I don't quite see what this has to do with a code
>  review apart from identifying such potentially buggy code (why write it
>  in such a way in the first place?).  After fixing it there's no point in
>  reviewing it over and over again, so there's really no reason for such
>  an ugly macro.

[Timo Kreuzer]

Warning in cases in which the compiler doesn't know whether something is
correct or not, is stupid in any case IMO, unless it's some
--enable-uber-pedantic-warnings compiler flag. It could as well say
"warning: your code could be wrong" and by chance this might be true.

Dmitry Gorbachev wrote:
>   Notice that the warning is "may be used
> uninitialized" and not "is
>  used uninitialized", so it is correct, in a sense.
>
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev
>
>

[Alex Ionescu]

No they are correct, most of the times the "may" means "given certain code
paths", not "may" as in "uhhh... I think it could happen?"

The reason the error often happens, with GCC *AND* MSVC (see previous e-mail) is stuff
like this:

BOOLEAN HaveYourDad;
PVOID Pen15;
ULONG YourMom;

if (YourMom)
{
     Pen15 = ExAllocateYourDad();
     HaveYourDad = TRUE;
}

....


if (HaveYourDad) ExReleaseYourDad(Pen15);

In this case, the compiler might say that your Pen15 may be used without having been
initialized because it doesn't realize that I'm only going to have your dad if I
also already had your mom.

Had you written:

if (YourMom) ExReleaseYourDad(Pen15); the compiler would probably be smart enough to
realize the side-effect.  

On 2009-11-25, at 3:31 PM, Timo Kreuzer wrote:

>   
>  Warning in cases in which the compiler doesn't know whether something is
>  correct or not, is stupid in any case IMO, unless it's some
>  --enable-uber-pedantic-warnings compiler flag. It could as well say
>  "warning: your code could be wrong" and by chance this might be true.
>  
>  Dmitry Gorbachev wrote:
> >   Notice that the warning is "may be used
> > uninitialized" and not "is
> >  used uninitialized", so it is correct, in a sense.
> >  
> >  _______________________________________________
> >  Ros-dev mailing list
> >  Ros-dev(a)reactos.org
> >  http://www.reactos.org/mailman/listinfo/ros-dev
> >  
> >   
>  
>  
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev 
Best regards,
Alex Ionescu

[Sarocet]

Alex Ionescu wrote:
>   No they are correct, most of the times the
> "may" means "given certain code paths", not "may" as in
> "uhhh... I think it could happen?"
>
>  The reason the error often happens, with GCC *AND* MSVC (see previous e-mail) is stuff
> like this:
>
>  BOOLEAN HaveYourDad;
>  PVOID Pen15;
>  ULONG YourMom;
>
>  if (YourMom)
>  {
>       Pen15 = ExAllocateYourDad();
>       HaveYourDad = TRUE;
>  }
>
>  ....
>
>
>  if (HaveYourDad) ExReleaseYourDad(Pen15);
>
>  In this case, the compiler might say that your Pen15 may be used without having been
> initialized because it doesn't realize that I'm only going to have your dad if I
> also already had your mom.
>
>  Had you written:
>
>  if (YourMom) ExReleaseYourDad(Pen15); the compiler would probably be smart enough to
> realize the side-effect.  
>     
You can't always simplify it by having a YourMum equivalent to HaveYourDad.
It's a pity that compilers don't provide a way to link Pen15 to
HaveYourDad. With such a hint they could realise that Pen15 is not being
used uninitialised there, while checking for usages not into a 
HaveYourDad conditional (as well as warn you if you set HaveYourDad and
Pen15 isn't set on that branch).

[Dmitry Gorbachev]

>   Warning in cases in which the compiler doesn't
> know whether something is
>  correct or not, is stupid in any case IMO, unless it's some
>  --enable-uber-pedantic-warnings compiler flag. 
But you can make a error a warning again, in newer GCCs.