This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] posix_spawn_file_actions_addopen needs to copy the path argument (BZ 17048)


This looks fine to me except for some trivia.  
Please put the BZ# in the subject line when you have one.

> 2014-06-11  Florian Weimer  <fweimer@redhat.com>
> 
> 	* posix/spawn_int.h (struct __spawn_action): Make the path string
> 	non-const to support deallocation.
> 
> 	* posix/spawn_faction_addopen.c
> 	(posix_spawn_file_actions_addopen): Make a copy of the pathname.
> 
> 	* posix/spawn_faction_destroy.c
> 	(posix_spawn_file_actions_destroy): Adjust comment.  Deallocate
> 	path in all spawn_do_open actions.
> 
> 	* posix/tst-spawn.c (do_test): Exercise the copy operation in
> 	posix_spawn_file_actions_addopen.

These are all one paragraph (i.e. no blank lines in between) when they are
all part of the same change.  Put [BZ #17048] at the top.

> +/* Deallocated the file actions.  */

s/Deallocated/Deallocate/

> +      struct __spawn_action *sa = file_actions->__actions + i;

I always have a mild preference to &foo[i] when that's what you're doing.


Thanks,
Roland


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]