commit "Langinfo: Implement CODESET on Windows" leaks memory

gcc 4.8.3 for Win64 complains about a warning from commit "Langinfo: 
Implement CODESET on Windows"

https://perl5.git.perl.org/perl.git/commitdiff/8d72e74e3dc5017c5a3fade48e0c74109c297ebc

---------------------------------------------
...\locale.c: In function 'S_my_nl_langinfo':
...\locale.c:2581:35: warning: ignoring return value of 
'S_save_to_buffer', declared with attribute warn_unused_result 
[-Wunused-result]
                      save_to_buffer("CP", &PL_langinfo_buf,
                                    ^
---------------------------------------------

This looks like leaked memory to me.

Why is 
https://perl5.git.perl.org/perl.git/blob/8d72e74e3dc5017c5a3fade48e0c74109c297ebc:/locale.c#l2555 
the retval of save_to_buffer() is not captured?
0
bulk88
3/14/2018 5:58:02 AM
perl.perl5.porters 47200 articles. 0 followers. Follow

5 Replies
48 Views

Similar Articles

[PageSpeed] 30

On 03/13/2018 11:58 PM, bulk88 wrote:
> gcc 4.8.3 for Win64 complains about a warning from commit "Langinfo:=20
> Implement CODESET on Windows"
>=20
> https://perl5.git.perl.org/perl.git/commitdiff/8d72e74e3dc5017c5a3fade4=
8e0c74109c297ebc=20
>=20
>=20
> ---------------------------------------------
> ..\locale.c: In function 'S_my_nl_langinfo':
> ..\locale.c:2581:35: warning: ignoring return value of=20
> 'S_save_to_buffer', declared with attribute warn_unused_result=20
> [-Wunused-result]
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 save_to_buffer("CP", &PL=
_langinfo_buf,
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ^
> ---------------------------------------------
>=20
> This looks like leaked memory to me.
>=20
> Why is=20
> https://perl5.git.perl.org/perl.git/blob/8d72e74e3dc5017c5a3fade48e0c74=
109c297ebc:/locale.c#l2555=20
> the retval of save_to_buffer() is not captured?
>=20

Thanks for spotting this, but it isn't a real error nor leak.  The=20
function call points PL_langinfo_buf at memory (allocated if necessary)=20
that it has placed the string "CP" into.buf, whose memory is freed at=20
the end of the program.  There is no need to look at the return value.=20
A few lines below the function is called again, and the return value is=20
retained.

The non-error return value of this function is always a pointer to=20
PL_langinfo_buf.  It saves a statement to do this.  I added the=20
warn_unused_result attribute to this static function to have the=20
compiler remind me that I need to update the return value of the calling=20
function, as I had failed to do this

This warning is only on Windows, and I didn't see it when I was testing=20
on Windows.

In commit 32a62865ef662fce2b2250a7e0eca15861e7fe20, I cast the return to=20
void.
0
public
3/14/2018 3:24:10 PM
Karl Williamson wrote:
> Thanks for spotting this, but it isn't a real error nor leak.  The 
> function call points PL_langinfo_buf at memory (allocated if necessary) 
> that it has placed the string "CP" into.buf, whose memory is freed at 
> the end of the program.  There is no need to look at the return value. A 
> few lines below the function is called again, and the return value is 
> retained.
> 
> The non-error return value of this function is always a pointer to 
> PL_langinfo_buf.  It saves a statement to do this.  I added the 
> warn_unused_result attribute to this static function to have the 
> compiler remind me that I need to update the return value of the calling 
> function, as I had failed to do this
> 
> This warning is only on Windows, and I didn't see it when I was testing 
> on Windows.
> 
> In commit 32a62865ef662fce2b2250a7e0eca15861e7fe20, I cast the return to 
> void.

That didn't work.

-------------------------------
C:\p527\srcnew\win32>b88dmake  WIN64=undef  mini/locale.o
if not exist ".\mini" mkdir ".\mini"
copy config_H.gc config.h
         1 file(s) copied.
rem. > .\mini\.exists
gcc -c  -I.\include -I. -I.. -DWIN32 -DPERLDLL -DPERL_CORE -s -O2 
-fwrapv -fno-s
trict-aliasing -DPERL_EXTERNAL_GLOB -DPERL_IS_MINIPERL -omini\locale.o 
...\local
e.c
...\locale.c: In function 'S_my_nl_langinfo':
...\locale.c:2581:21: warning: ignoring return value of 
'S_save_to_buffer', decla
red with attribute warn_unused_result [-Wunused-result]
                      (void) save_to_buffer("CP", &PL_langinfo_buf,
                      ^

C:\p527\srcnew\win32>
-------------------------------

Maybe the retval should be removed completely and the pointer returned 
in the char ** it came in on, or only a "char * buf" arg and a "buf = 
S_save_to_buffer(buf);" thing for realloc situation. IDK. I didnt design 
this API.
0
bulk88
3/14/2018 6:30:05 PM
On 03/14/2018 12:30 PM, bulk88 wrote:
> Karl Williamson wrote:
>> Thanks for spotting this, but it isn't a real error nor leak.=C2=A0 Th=
e=20
>> function call points PL_langinfo_buf at memory (allocated if=20
>> necessary) that it has placed the string "CP" into.buf, whose memory=20
>> is freed at the end of the program.=C2=A0 There is no need to look at =
the=20
>> return value. A few lines below the function is called again, and the=20
>> return value is retained.
>>
>> The non-error return value of this function is always a pointer to=20
>> PL_langinfo_buf.=C2=A0 It saves a statement to do this.=C2=A0 I added =
the=20
>> warn_unused_result attribute to this static function to have the=20
>> compiler remind me that I need to update the return value of the=20
>> calling function, as I had failed to do this
>>
>> This warning is only on Windows, and I didn't see it when I was=20
>> testing on Windows.
>>
>> In commit 32a62865ef662fce2b2250a7e0eca15861e7fe20, I cast the return=20
>> to void.
>=20
> That didn't work.
>=20
> -------------------------------
> C:\p527\srcnew\win32>b88dmake=C2=A0 WIN64=3Dundef=C2=A0 mini/locale.o
> if not exist ".\mini" mkdir ".\mini"
> copy config_H.gc config.h
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 1 file(s) copied.
> rem. > .\mini\.exists
> gcc -c=C2=A0 -I.\include -I. -I.. -DWIN32 -DPERLDLL -DPERL_CORE -s -O2=20
> -fwrapv -fno-s
> trict-aliasing -DPERL_EXTERNAL_GLOB -DPERL_IS_MINIPERL -omini\locale.o=20
> ..\local
> e.c
> ..\locale.c: In function 'S_my_nl_langinfo':
> ..\locale.c:2581:21: warning: ignoring return value of=20
> 'S_save_to_buffer', decla
> red with attribute warn_unused_result [-Wunused-result]
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 (void) save_to_buffer("C=
P", &PL_langinfo_buf,
>  =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ^
>=20
> C:\p527\srcnew\win32>
> -------------------------------
>=20
> Maybe the retval should be removed completely and the pointer returned=20
> in the char ** it came in on, or only a "char * buf" arg and a "buf =3D=
=20
> S_save_to_buffer(buf);" thing for realloc situation. IDK. I didnt desig=
n=20
> this API.
>=20

Ah, this is a gcc compiler which is broken in regard to this attribute,=20
and they stubbornly insist that it should work the way it does.
I'll push a different approach
0
public
3/14/2018 6:37:32 PM
On 03/14/2018 12:37 PM, Karl Williamson wrote:
> On 03/14/2018 12:30 PM, bulk88 wrote:
>> Karl Williamson wrote:
>>> Thanks for spotting this, but it isn't a real error nor leak.=C2=A0 T=
he=20
>>> function call points PL_langinfo_buf at memory (allocated if=20
>>> necessary) that it has placed the string "CP" into.buf, whose memory=20
>>> is freed at the end of the program.=C2=A0 There is no need to look at=
 the=20
>>> return value. A few lines below the function is called again, and the=
=20
>>> return value is retained.
>>>
>>> The non-error return value of this function is always a pointer to=20
>>> PL_langinfo_buf.=C2=A0 It saves a statement to do this.=C2=A0 I added=
 the=20
>>> warn_unused_result attribute to this static function to have the=20
>>> compiler remind me that I need to update the return value of the=20
>>> calling function, as I had failed to do this
>>>
>>> This warning is only on Windows, and I didn't see it when I was=20
>>> testing on Windows.
>>>
>>> In commit 32a62865ef662fce2b2250a7e0eca15861e7fe20, I cast the return=
=20
>>> to void.
>>
>> That didn't work.
>>
>> -------------------------------
>> C:\p527\srcnew\win32>b88dmake=C2=A0 WIN64=3Dundef=C2=A0 mini/locale.o
>> if not exist ".\mini" mkdir ".\mini"
>> copy config_H.gc config.h
>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 1 file(s) copied.
>> rem. > .\mini\.exists
>> gcc -c=C2=A0 -I.\include -I. -I.. -DWIN32 -DPERLDLL -DPERL_CORE -s -O2=
=20
>> -fwrapv -fno-s
>> trict-aliasing -DPERL_EXTERNAL_GLOB -DPERL_IS_MINIPERL -omini\locale.o=
=20
>> ..\local
>> e.c
>> ..\locale.c: In function 'S_my_nl_langinfo':
>> ..\locale.c:2581:21: warning: ignoring return value of=20
>> 'S_save_to_buffer', decla
>> red with attribute warn_unused_result [-Wunused-result]
>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 (void) save_to_buf=
fer("CP", &PL_langinfo_buf,
>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ^
>>
>> C:\p527\srcnew\win32>
>> -------------------------------
>>
>> Maybe the retval should be removed completely and the pointer returned=
=20
>> in the char ** it came in on, or only a "char * buf" arg and a "buf =3D=
=20
>> S_save_to_buffer(buf);" thing for realloc situation. IDK. I didnt=20
>> design this API.
>>
>=20
> Ah, this is a gcc compiler which is broken in regard to this attribute,=
=20
> and they stubbornly insist that it should work the way it does.
> I'll push a different approach
>=20

Is it fixed now in the smoke-me/khw-locale branch?
0
public
3/20/2018 8:13:49 PM
Karl Williamson wrote:
>> Ah, this is a gcc compiler which is broken in regard to this 
>> attribute, and they stubbornly insist that it should work the way it 
>> does.
>> I'll push a different approach
>>
> 
> Is it fixed now in the smoke-me/khw-locale branch?

Yes, warning silenced.
0
bulk88
3/21/2018 6:07:58 AM
Reply: