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

[Peter Wang]

On Thu, 24 Apr 2025 19:52:09 +1000 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> 
> > 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.
> 

Done both.

> > +{
> > +    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.

I've simplified the function so it aborts.

(The previous implementation also would return 0 on truncation,
which wasn't the intention.)

Peter