On 04/07/2020 09:56, Emmanuel Deloget wrote:
On Fri, Jul 3, 2020 at 9:15 PM Tito <farmat...@tiscali.it> wrote:
On 7/3/20 6:13 PM, Xabier Oneca -- xOneca wrote:
Also, there's a warning compiling this function: return discards
‘const’ qualifier from pointer target type
Hi,
cast it?
char* FAST_FUNC last_char_is(const char *s, int c)
{
if (!s || !*s) return NULL;
while (*(++s));
return (*(--s) == c) ? (char *)s : NULL;
}
In favor of const-correctness, wouldn't it be better to return a const
char*? It's quite strange to promote a string to const and to a part
of it that is non-const (not to mention that if you feed the function
with a real const char* then the cast is not a good thing to do).
Existing callers rely on the return type being char*.
This is really the same problem as with standard library functions such
as strchr(). The function should have a return type of char* if its
argument has type char*. The function should have a return type of const
char* if its argument has type const char*. The current signature is the
only signature that allows all code that should be valid, at the expense
of not rejecting code that should be invalid.
busybox could define last_char_is as a macro that wraps this function,
which would optionally cast the return value to const char*. Such a
macro would be easy to define using _Generic, assuming the compiler is
recent enough to support it. This would ensure all code that should be
valid remains valid, while some (not all) code that should not be valid
becomes invalid. Such a macro could look like
#define last_char_is(s, c) \
(_Generic((s), \
const char *: (const char *) (last_char_is) ((s), (c)), \
default: (last_char_is) ((s), (c))))
But considering busybox uses standard library functions with the same
problem throughout the codebase, fixing it just for last_char_is while
leaving alone all other functions may not be worth the effort.
Cheers,
Harald van Dijk
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
