Re: [RFC v2 PATCH 1/3] init: add sys-wrapper.h

[Namhyung Kim]

On Mon, Aug 30, 2010 at 21:11, Arnd Bergmann <arnd@xxxxxxxx> wrote:
> On Sunday 29 August 2010, Namhyung Kim wrote:
>> +
>> +/* These macro are called just before/after actual syscalls. */
>> +#define KSYS_PREPARE                         \
>> +     mm_segment_t old_fs = get_fs();         \
>> +     set_fs(KERNEL_DS);
>> +
>> +#define KSYS_RESTORE                         \
>> +     set_fs(old_fs);
>
> These macros are not that nice, because they depend on context.
> I would probably open-code them in each function, or possibly
> use a single macro to combine it to something like
>
> #define kern_sys_call(call, ...)        \
> ({                                      \
>        mm_segment_t old_fs = get_fs(); \
>        long result;                    \
>        set_fs(KERNEL_DS);              \
>        result = call(__VA_ARGS__);     \
>        set_fs(old_fs);                 \
>        result;                         \
> })
>
> static inline int kern_sys_link(const char *oldname, const char *newname)
> {
>        return kern_sys_call(sys_link, (const char __user __force *)oldname,
>                             (const char __user __force *)newname);
> }
>

Cool. Will use it. :-)


>> +static inline int kern_sys_fchown(unsigned int fd, uid_t user, gid_t group)
>> +{
>> +     int ret;
>> +     KSYS_PREPARE;
>> +
>> +     ret = sys_fchown(fd, user, group);
>> +
>> +     KSYS_RESTORE;
>> +     return ret;
>> +}
>
> When there are no pointer arguments, there is no need to do set_fs
> tricks.
>

My intentions was it might be good, IMHO, if we have common setup/tear-down code
around actual syscall possibly extended in future. But now I think
it's a kind of over-
engineering so I'll discard it and follow your advice above.

Thanks.


-- 
Regards,
Namhyung Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/