[m-rev.] for review: Fix incorrect uses of strncat.

[Zoltan Somogyi]

On Thu, 24 Apr 2025 17:18:17 +1000, Peter Wang <novalazy at gmail.com> wrote:
> Fix incorrect uses of strncat.
> 
> The size argument to strncat() gives the number of characters to copy
> from the input string, *not* the size of the destination buffer.

Thanks for catching that.

> diff --git a/runtime/mercury_deep_profiling.c b/runtime/mercury_deep_profiling.c
> index 75a3b1e41..c71e7a111 100644
> --- a/runtime/mercury_deep_profiling.c
> +++ b/runtime/mercury_deep_profiling.c
> @@ -374,6 +374,27 @@ static  FILE        *debug_fp;
>  
>  #define MR_FILENAME_BUF_LEN     1024
>  
> +// Append the string pointed to by src into the buffer pointed to by dst.
> +// sz is the size of the destination buffer. If there is not enough space in
> +// the destination buffer, the source string will be truncated to fit,
> +// and the destination string will be null-terminated.
> +//
> +// Returns 0 on success, or -1 if there was insufficient space.
> +static int
> +safecat(char dst[], size_t sz, const char *src)

We usually add an MR_ prefix to the names of even static
functions in the runtime. It is not necessary, but looks more
consistent.

I would give sz a more meaningful name, such as dest_buffer_size.
The job of the function is to correct a misunderstanding of the role
of this argument, after all.

> +{
> +    size_t dst_len = strnlen(dst, sz);
> +    if (dst_len >= sz)
> +        return -1;

Add {} around the then part.

However, even though you declared safecat to return an int,
that int is ignored at the call sites, so returning -1 instead of 0
does nothing. Given that the call sites *all* call this function
with a string whose length is a fixed constant, I think that
an abort would be the right response here, if the relationship
between sz and the length of src is ever screwed up in the future.
That would allow making this function return void.

The diff is otherwise fine. Thanks again.

Zoltan.