Re: [LTP] [PATCH] tst_module: create new library functions for kernel modules

[Mike Frysinger <vapier@gentoo.org>]

[-- Attachment #1.1: Type: Text/Plain, Size: 1494 bytes --]

On Thursday 20 June 2013 07:54:37 Alexey Kodanev wrote:
> +/*
> + * Check module existence.
> + *
> + * @res_path: if this pointer isn't NULL, found module's path will be
> + * written to it.
> + *
> + * In case of failure, test'll call cleanup_fn and exit with TCONF return
> + */
> +void tst_module_exist(void (*cleanup_fn)(void), const char *mod_name,
> +	char *res_path, size_t max_path);

i hate APIs like this.  they're so 1990 :P.

look at the getline() func and behave like that instead (take char **res_path 
and size_t *max_path instead)

> +/* declared in tst_tmpdir.c */
> +const char *tst_get_startwd(void);

use a proper header instead.  extern func decls only leads to bit rot and hard 
to debug crashes.

> +	char buf[PATH_MAX];

PATH_MAX should be taken out back and shot.  there's no reason to use it 
anymore when we have things like asprintf().  plus, it isn't portable -- 
there's no guarantee it'll be defined.  i know the irony complaining about 
portability with PATH_MAX when asprintf() isn't in POSIX either, but the 
difference is, we have an asprintf() fallback already in lib/, and we have code 
using it today.

> +	char cmd[strlen(mod_path) + strlen(params) + 9];
> +	snprintf(cmd, sizeof(cmd), "insmod %s %s", mod_path, params);
> +
> +	if (system(cmd) != 0) {

use fork()+execvp() instead.  there's no need to normalize to a string and 
then run through a shell.

wonder if we should add a util func for this ...
-mike

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 184 bytes --]

------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev

[-- Attachment #3: Type: text/plain, Size: 155 bytes --]

_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list