Re: [PATCH] libkmod: add finit_module logic

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tuesday 22 January 2013 19:24:54 Kees Cook wrote:

just nits ...

> +int kmod_file_get_direct(const struct kmod_file *file)
> +{
> +	return file->direct;
> +}
> +
> +int kmod_file_get_fd(const struct kmod_file *file)
> +{
> +	return file->fd;
> +}

these are only in the private header, so they could be static inlines since 
you don't have to worry about the ABI layout of the structure

> --- a/libkmod/libkmod-module.c
> +++ b/libkmod/libkmod-module.c
> 
> +#ifndef __NR_finit_module
> +# if defined(__x86_64__)
> +#  define __NR_finit_module 313
> +# elif defined(__i386__)
> +#  define __NR_finit_module 350
> +# elif defined(__arm__)
> +#  define __NR_finit_module 379
> +# endif
> +#endif
> +
> +#ifdef __NR_finit_module
> +static inline int finit_module(int fd, const char *uargs, int flags)
> +{
> +   return syscall(__NR_finit_module, fd, uargs, flags);
> +}
> +#else
> +static inline int finit_module(int fd, const char *uargs, int flags)
> +{
> +   errno = ENOSYS;
> +   return -1;
> +}
> +#endif

in the "ifndef __NR_finit_module" logic, you could add an:
# else
#  define __NR_finit_module -1

then you would only have one finit_module() implementation to content with

you should add a configure test for the finit_module function.  when (if?) glibc 
starts including it, you'll want to avoid the fallback definition.

> --- a/testsuite/init_module.c
> +++ b/testsuite/init_module.c
> 
> +TS_EXPORT int finit_module(const int fd, const char *args, const int
> flags);
> +
> +int finit_module(const int fd, const char *args, const int flags)

is the double decl existing style ?  i don't think it's needed:

TS_EXPORT int finit_module(const int fd, const char *args, const int flags)
{
	...
}

> +	mem = mmap(0, len, PROT_READ, MAP_PRIVATE, fd, 0);

first arg should be NULL
-mike

Attachment: signature.asc
Description: This is a digitally signed message part.


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux