Re: [PATCH 1/3] src: fix build warnings

[Mikael Magnusson <mikachu@xxxxxxxxx>]

On 1/2/21, Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx> wrote:
> Felipe Contreras wrote on Thu, 31 Dec 2020 05:41 +00:00:
>>   utils.c: In function ‘getkeystring’:
>>   cc1: warning: function may return address of local variable
>> [-Wreturn-local-addr]
>>   utils.c:6703:16: note: declared here
>>    6703 |     char *buf, tmp[1];
>>         |                ^~~
>>
>> diff --git a/Src/utils.c b/Src/utils.c
>> index 5151b89a8..37ae4c854 100644
>> --- a/Src/utils.c
>> +++ b/Src/utils.c
>> @@ -6700,7 +6700,8 @@ ucs4toutf8(char *dest, unsigned int wval)
>>  mod_export char *
>>  getkeystring(char *s, int *len, int how, int *misc)
>>  {
>> -    char *buf, tmp[1];
>> +    static char tmp[1];
>> +    char *buf;
>
> The docstring explicitly promises the returned string will be newly-
> allocated from the heap, so this change makes the implementation
> inconsistent with the docstring.

If we ever return the address of tmp, bad things are likely to happen
regardless of the static so hopefully that never happens. Returning
the address of a local variable basically means it will be junk
(current hypothetical behavior), whereas returning a static means the
function wouldn't be re-entrant (hypothetical behavior with the
patch). I don't think any reading of "newly allocated from the heap"
would include a location on a popped stack though, so I don't see how
the docstring enters into it at all.

-- 
Mikael Magnusson