[PATCH] Correctly use execlp to specify module image name.

[Trevor Saunders]

On Fri, Mar 18, 2011 at 10:58:12PM +0100, Boris Du?ek wrote:
> The first argument to the execlp function is the filename of the file
> containing the executable image that will be loaded by the dynamic
> loader. The second argument to the function then seems redundant, since
> it should be the same as the first argument (since it is what gets
> passed as argv[0] to the resulting program). But it can be different
> from the first argument, e.g. when launching a shell script: then the
> first argument will be e.g. /bin/sh, since that is the executable image
> file from the view of the operating system, but the second argument
> (i.e. argv[0]) will be the name of the shell script (e.g. script.sh).
> Therefore it makes sense to specify the same for the first and second
> argument when no interpreter is in place.
> 
> This fixes display using ps aux - the module processes launched by
> speech-dispatcher main process had empty COMMAND field, making them
> hardly identifiable for user inspecting running processes.

yes, this should make debugging modules a bit more convienent, thanks
for getting around to writing this patch :) I'd been meaning to for a
while but never got to it :(

> ---
>  src/server/module.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/server/module.c b/src/server/module.c
> index dc60f87..9fe6a6f 100644
> --- a/src/server/module.c
> +++ b/src/server/module.c
> @@ -216,12 +216,12 @@ OutputModule *load_output_module(char *mod_name, char 
> *mod_prog,
>               }
>  
>               if (cfg == 0) {

My cache is pretty cold here, but could you just arrange for arg1 to
point to "" until a real file name is set to get rid of this if?

> -                     if (execlp(module->filename, "", (char *)0) == -1) {
> +                     if (execlp(module->filename, module->filename, (char 
> *)0) == -1) {

if isn't needed, there is no such thing as a successful return from
exec, if we get back here its an error.

>                               exit(1);
>                       }
>               } else {
> -                     //if (execlp("valgrind", "" ,"--trace-children=yes", 
> module->filename, arg1, arg2, (char *) 0) == -1){

I have no idea why that cruft is there, but as far as I can see there is
no reason for it and it should just go away while your touching this.

> -                     if (execlp(module->filename, "", arg1, (char *)0) == 
> -1) {
> +                     //if (execlp("valgrind", "valgrind" 
> ,"--trace-children=yes", module->filename, arg1, arg2, (char *) 0) == -1){
> +                     if (execlp(module->filename, module->filename, arg1, 
> (char *)0) == -1) {

same, no need for the if

>                               exit(1);
>                       }
>               }

Thanks!

Trev

> -- 
> 1.7.4
> 
> 
> _______________________________________________
> Speechd mailing list
> Speechd at lists.freebsoft.org
> http://lists.freebsoft.org/mailman/listinfo/speechd
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: 
<http://lists.freebsoft.org/pipermail/speechd/attachments/20110318/f1099b79/attachment.pgp>