Re: [Libguestfs] [nbdkit PATCH v2] plugin: add and use nbdkit_realpath

On 02/21/2018 11:13 AM, Pino Toscano wrote: > On Wednesday, 14 February 2018 18:06:10 CET Eric Blake wrote: >> On 02/14/2018 10:53 AM, Pino Toscano wrote: >>> Introduce a new helper function to resolve a path name, calling >>> nbdkit_error on failure: other than doing what nbdkit_absolute_path >>> does, it also checks that the file exists (and thus avoids errors later >>> on).  To help distinguish it from nbdkit_absolute_path, improve the >>> documentation of the latter. >>> >>> Apply it where an existing path is required, both in nbdkit itself and >>> in plugins. >>> >>> Related to: https://bugzilla.redhat.com/show_bug.cgi?id=1527334 >>> --- >>> +char * >>> +nbdkit_realpath (const char *path) >>> +{ >>> +  char *ret; >>> + >>> +  if (path == NULL || *path == '\0') { >>> +    nbdkit_error ("cannot resolve a null or empty path"); >>> +    return NULL; >>> +  } >>> + >>> +  ret = realpath (path, NULL); >> >> Wait. Does this even work? > > It works in the same way as nbdkit_absolute_path() did: when calling it > on a relative path, nbdkit_absolute_path() will prepend $PWD to it, > while the new nbdkit_realpath() will do something similar. My question is whether the $PWD it is prepending is guaranteed to be correct.  If we call this too late, then it is not; it would be trivial to rewrite it so that we manually prepend something cached from when the program started, so that this is then conceptually correct no matter when we change directories later. > > At least the usages that I changed are called before start_serving() > (and thus before fork_into_background()), so I think there should be no > issue wrt paths. Oh. Hmm.  Interesting observation, that .config and .config_complete finish PRIOR to start_serving() (and makes sense, too, as if we're going to diagnose any errors, printing to stderr is a LOT easier at the point where we haven't forked and redirected stderr).  But maybe that means that we should document that these two functions are available for plugins/filters, but ONLY for use during .config/.config_complete; that any time later they are unsafe.  OR, we can fix them to ALWAYS work, even if they are invoked somewhere outside of .config. > > Did I miss anything? > No, but I missed that .config runs before forking.