Ralf Baechle wrote:
> On Mon, Nov 05, 2007 at 10:51:43PM +0100, Franck Bui-Huu wrote:
>
>>> Memset is almost always only ever invoked with a zero argument. So the
>>> idea was to have something like this:
>>>
>>> extern void *__memset(void *__s, int __c, size_t __count);
>>> extern void *bzero(void *__s, size_t __count);
>>>
>>> static inline void *memset(void *s, int c, size_t count)
>>> {
>>> if (__builtin_constant_p(c) && c == 0) {
>>> bzero(s, count);
>>> return s;
>>> } else
>>> return __memset(s, __c, count);
>>> }
>>>
>>> But that was never quite implemented like this as you noticed.
>> Well I'm not sure we really need this. bzero() is not part of the
>> Linux string API, so it can only be used by MIPS specific code. And
>> with the current implementation of bzero(), $a1 needs to be setup to 0
>> anyway. That's why I simply killed it...
>>
>> BTW, can memset() be an inlined function ?
>
> It can be anything, macro, inline or outline function. In the kernel
> there are fewer restrictions than for a standards compliant library in
> userspace.
>
> You may take the i386 implementation in include/asm-x86/string_32.h as
> an extreme example.
>
> Older gcc used to generate significantly worse code for inline functions
> than for macros so Linux became a fairly excessive user of macros. This
> has very much improved since, so these days inlines are prefered over
> macros where possible.
Yes but ISTR that gcc generates some calls to memset and since
builtin functions are disabled the final link failed if memset
is inlined. I'll try to reproduce...
>
>> Yes I noticed this. Actually I'm wondering if we couldn't add a new
>> function, fill_user() like the following:
>>
>> extern size_t fill_user(void __user *to, int c, size_t len);
>
> That's much better function name than the old __bzero - except that
Actually I named it '__fill_user', since it doesn't call access_ok().
> __bzero effectivly took a long argument for the 2nd argument so 32-bit
> on 32-bit kernels and 64-bit on 64-bit kernels.
Isn't size_t meaning ?
Perhaps in this case __kernel_size_t is better...
>
>> This could be used by both memset() and clear_user():
>>
>> #define memset(s,c,l) ({ (void)fill(s,c,l); s; })
>> #define clear_user(t,l) fill_user(t,0,l)
>>
>> Therefore the definition of clear_user() could be saner.
>
> Looks alot nicer that way though an inline is probably preferable as
> expressed above.
>
Yes I have a patchset which clean up a bit uaccess.h and does this but
it's under construction. It actually tries to convert all macros into
inlines and the file is much more readable and as a bonus side we could
easily add __must_check annotations which are currently missing.
I'll try to finish it this week but in the meantime can we just kill
__bzero or do you want me to include it in the future patchset ?
Franck
