On 2021/7/16 11:34, Theodore Y. Ts'o wrote:
> On Wed, Jun 30, 2021 at 04:27:17PM +0800, wuguanghao wrote:
>> In ss_create_invocation(), it is necessary to check whether
>> returned by malloc is a null pointer.
>>
>> Signed-off-by: Wu Guanghao <wuguanghao3@xxxxxxxxxx>
>> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@xxxxxxxxxx>
>> ---
>> lib/ss/invocation.c | 38 ++++++++++++++++++++++++++++++++------
>> 1 file changed, 32 insertions(+), 6 deletions(-)
>>
> Instead of adding all of these goto targets (which is fragile if for
> some reason the code gets rearranged), it would be better to make sure
> everything that we might want to free is initialized, i.e.:
>
> register ss_data *new_table = NULL;
> register ss_data **table = NULL;
>
> new_table = (ss_data *) malloc(sizeof(ss_data));
> if (!new_table)
> goto out;
> memset(new_table, 0, sizeof(ss_data));
>
> and then exit path can just look like this:
>
> out:
> if (new_table) {
> free(new_table->prompt);
> free(new_table->info_dirs);
> }
> free(new_table);
> free(table);
> *code_ptr = ENOMEM;
> return 0;
>
> ... which is much cleaner, and means in the future, you don't need to
> figure out which goto label you might need to jump to.
>
> Cheers,
>
> - Ted
>
> P.S. And if we are making all of these changes to the function's
> initializers, it might be a good time to zap the "register" keywords
> for any lines we are changing, or are nearby, while we're at it.
>
> .
Thanks for your suggestion.
We will rewrite the patch and remove the "register" keywords.
Regards
Zhiqiang Liu
.
