This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] posix: New function posix_spawn_file_actions_addchdir_np [BZ #17405]
* Adhemerval Zanella:
> On 26/10/2018 11:07, Florian Weimer wrote:
>> I'm not adding documentation in this patch because none exists for the
>> posix_spawn functionality.
>>
>> I initially copied posix/spawn_faction_addchdir.c from
>> posix/spawn_faction_addopen.c, hence the backdated copyright notice.
>>
>> If we agree that this is the way to go, I will add further such
>> functions (fchdir, chroot, perhaps more).
>
> Do we have a position from Austin Group if this will be added on
> standard eventually? From the issue report [1], it seems that
> posix_spawn_file_actions_addchdir with similar signature seems the one
> proposed, but it is still marked as opened.
>
> Even though, I do think this is a good addition and help narrow down that
> functionality posix_spawn lacks.
>
> [1] http://austingroupbugs.net/view.php?id=1208
I would prefer to keep the _np prefix in case the proposed interface is
altered in some way.
I didn't realize (or had forgotten)that Solaris implemented this,
thanks. As far as I can tell based on their manual page, my
implementation matches theirs.
>> +* The posix_spawn_file_actions_addchdir_np function has been added, enabling
>> + posix_spawn and posix_spawnp to run the new process in a different
>> + directory.
>
> It think we should mention this is a GNU extension, although Solaris
> 11 also provided the same extension.
Good point, I will mention both:
* The posix_spawn_file_actions_addchdir_np function has been added,
enabling posix_spawn and posix_spawnp to run the new process in a
different directory. This is a GNU extension and similar to the
Solaris function of the same name.
>> diff --git a/posix/spawn_faction_addchdir.c b/posix/spawn_faction_addchdir.c
>> new file mode 100644
>> index 0000000000..d719fe4358
>> --- /dev/null
>> +++ b/posix/spawn_faction_addchdir.c
>> @@ -0,0 +1,52 @@
>
> Missing one line description.
Yeah, right. That happens when you copy the header from older
templates.
What about this?
/* Add a directory change to a file action list for posix_spawn.
>> +++ b/posix/tst-spawn-chdir.c
>> @@ -0,0 +1,115 @@
>
> One line description.
/* Test the posix_spawn_file_actions_addchdir_np function.
>> +static int
>> +do_test (void)
>> +{
>> + char *directory = support_create_temp_directory ("tst-spawn-chdir-");
>> + {
>> + /* Avoid symbolic links, to get more consistent behavior from the
>> + pwd command. */
>> + char *tmp = realpath (directory, NULL);
>> + if (tmp == NULL)
>> + FAIL_EXIT1 ("realpath: %m");
>> + free (directory);
>> + directory = tmp;
>> + }
>> + const char *output_file = "output-file";
>> + char *output_file_path = xasprintf ("%s/%s", directory, output_file);
>> + add_temp_file (output_file_path);
>> +
>> + posix_spawn_file_actions_t actions;
>> + TEST_COMPARE (posix_spawn_file_actions_init (&actions), 0);
>
> Maybe is it worth to add an extra posix_spawn_file_actions_addopen
> operation to check if posix_spawn_file_actions_addchdir_np honors the
> issue order?
Hmm, I have to think about it. Would you consider mere creation of a
file in the expected directory consider sufficient evidence that the
directory changes happen as expected?
Thanks,
Florian