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: 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


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