Re: [PATCH] ln: alloc pointer variables could be deleted initialized, it first allocates memory

[Pádraig Brady]

On 05/08/2022 08:00, Li kunyu wrote:
> The alloc pointer variable does not need to be initialized and assigned
> NULL, it has allocated memory before use, which seems to reduce a mov
> instruction.
>
> Signed-off-by: Li kunyu <kunyu@nfschina.com>
> ---
>   src/ln.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/ln.c b/src/ln.c
> index bb4695853..cf5d69a64 100644
> --- a/src/ln.c
> +++ b/src/ln.c
> @@ -352,17 +352,17 @@ do_link (char const *source, int destdir_fd, char const 
> *dest_base,
>             if (backup_base)
>               {
>                 char *backup = backup_base;
> -              void *alloc = NULL;
> +              void *alloc;
>                 ptrdiff_t destdirlen = dest_base - dest;
>                 if (0 < destdirlen)
>                   {
>                     alloc = xmalloc (destdirlen + strlen (backup_base) + 1);
>                     backup = memcpy (alloc, dest, destdirlen);
>                     strcpy (backup + destdirlen, backup_base);
> +                 free (alloc);
>                   }
>                 quoted_backup = quoteaf_n (2, backup);
>                 backup_sep = " ~ ";
> -              free (alloc);
>               }
>             printf ("%s%s%s %c> %s\n", quoted_backup, backup_sep,
>                     quoteaf_n (0, dest), symbolic_link ? '-' : '=',

This doesn't look correct.
The memcpy() resets "backup" to the allocated area,
which is then referenced in the quoteaf_n() call.
Also in general, it's easier to reason about allocations
when they're declared and free'd in the same scope.

thanks,
Pádraig