Re: [PATCH 2/2] MdeModulePkg/UdfDxe: suppress incorrect compiler warning in ReadFile()

["Zeng, Star" <star.zeng@intel.com>]

It seems generic compiler issue to have this kind of false alarm.
And it seems hard decision to disable the warning for all the compilers.
We'd better to fix the build failure first, I will submit patch that will use similar method with this patch.


Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com] 
Sent: Thursday, September 14, 2017 4:20 PM
To: Zeng, Star <star.zeng@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; Gao, Liming <liming.gao@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; edk2-devel-01 <edk2-devel@lists.01.org>; Paulo Alcantara <pcacjr@zytor.com>
Subject: Re: [edk2] [PATCH 2/2] MdeModulePkg/UdfDxe: suppress incorrect compiler warning in ReadFile()

On 09/14/17 03:20, Zeng, Star wrote:
> Seemingly, VS has similar issue with GCC.
> 
> VS2010/VS2012 still have the building failures below after this patch. 
> :(
> edk2\mdemodulepkg\universal\disk\udfdxe\filesystemoperations.c(1083) : 
> error C2220: warning treated as error - no 'executable' file generated
> edk2\mdemodulepkg\universal\disk\udfdxe\filesystemoperations.c(1083) : 
> warning C4701: potentially uninitialized local variable 'FilePosition' 
> used
> edk2\mdemodulepkg\universal\disk\udfdxe\filesystemoperations.c(1078) : 
> warning C4701: potentially uninitialized local variable 
> 'FinishedSeeking' used
> edk2\mdemodulepkg\universal\disk\udfdxe\filesystemoperations.c(1167) : 
> warning C4701: potentially uninitialized local variable 'Data' used
> edk2\mdemodulepkg\universal\disk\udfdxe\filesystemoperations.c(1167) : 
> warning C4703: potentially uninitialized local pointer variable 'Data' 
> used
> 
> 
> Liming, Dandan and Hao,
> Do you remember how we fix this kind of false alarm before?
> Just initialize the variable at the beginning of the function?

I think that all such warnings should be evaluated carefully.

Can you please file a TianoCore BZ about the above? Paulo might want to take a look.

Thanks!
Laszlo


> -----Original Message-----
> From: Zeng, Star
> Sent: Thursday, September 14, 2017 8:43 AM
> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo Ersek 
> <lersek@redhat.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; 
> edk2-devel-01 <edk2-devel@lists.01.org>; Gao, Liming 
> <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH 2/2] MdeModulePkg/UdfDxe: suppress 
> incorrect compiler warning in ReadFile()
> 
> Comparing adding workaround in code with suppressing it in *OLD* version GCCs, I prefer the latter personally.
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of 
> Ard Biesheuvel
> Sent: Thursday, September 14, 2017 2:52 AM
> To: Laszlo Ersek <lersek@redhat.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; 
> edk2-devel-01 <edk2-devel@lists.01.org>; Gao, Liming 
> <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH 2/2] MdeModulePkg/UdfDxe: suppress 
> incorrect compiler warning in ReadFile()
> 
> On 13 September 2017 at 11:49, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 09/13/17 08:43, Zeng, Star wrote:
>>> Beyond the Rb (I do not want to block this patch series), I am curious about one question.
>>>
>>> There may be more this kind of workarounds to fix the build failure.
>>> Is it possible to disable the warning (like below example for VS) for specific version of GCC for this kind of false alarm?
>>>
>>>
>>> ProcessorBind.h:
>>> #if defined(_MSC_EXTENSIONS)
>>>
>>> ...
>>>
>>> #if _MSC_VER == 1800 || _MSC_VER == 1900
>>>
>>> //
>>> // Disable these warnings for VS2013.
>>> //
>>>
>>> //
>>> // This warning is for potentially uninitialized local variable, and 
>>> it may cause false // positive issues in VS2013 and VS2015 build // 
>>> #pragma warning ( disable : 4701 )
>>>
>>> //
>>> // This warning is for potentially uninitialized local pointer 
>>> variable, and it may cause // false positive issues in VS2013 and
>>> VS2015 build // #pragma warning ( disable : 4703 )
>>>
>>> #endif
>>>
>>> #endif
>>
>> I think starting with gcc-4.6, gcc supports the "diagnostics" pragma, 
>> which can be used to suppress warnings.
>>
>> Unfortunately, there's no pragma to suppress *only* the incorrect 
>> warnings :) So if we set the pragma, we could lose even those 
>> warnings that point out real bugs.
>>
> 
> That applies to the VS case as well. But I think doing this for older GCCs is fine, most EDK2 developers use a newer version anyway, so we will not lose any coverage by doing so.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>