On 04/12/2012 09:41 AM, Kevin Wolf wrote:
> Am 12.04.2012 15:17, schrieb Jeff Cody:
>>>> + *
>>>> + * Returns NULL if no relative or absolute path can be found.
>>>> + */
>>>> +static char *path_find_relative(char *current, char *backing)
>>>> +{
>>>> + char *src;
>>>> + char *dest;
>>>> + char *src_tmp;
>>>> + char *src_dir;
>>>> + char *rel_backing = NULL;
>>>> + char relpath[PATH_MAX] = {0};
>>>> + int offset = 0;
>>>> +
>>>> +
>>>> + src = realpath(current, NULL);
>>>
>>> My POSIX manpage says:
>>>
>>> "If resolved_name is a null pointer, the behavior of realpath() is
>>> implementation-defined."
>>>
>>> It seems to have been standardised by now, but I'm not sure if it is a
>>> good idea to rely on a quite new feature on some OSes.
>>>
>>
>> As the comments pointed out by Eric and Paolo indicate, the issue is
>> worse on some platforms. It would be nice to be able to rely on
>> canonicalize_file_name(), so I like Daniel's suggestion of pulling in
>> Paolo's proposed glib patch into QEMU. Any objections to bundling
>> Paolo's patch with my next (v1) submission?
>
> I haven't looked at that patch yet, but if it's licensed appropriately
> (IIUC, Paolo is not the only copyright owner), I think we can pull it in.
>
>>>> + }
>>>> +
>>>> + src_tmp = g_strdup(src);
>>>> +
>>>> + if (!strcmp(backing, dest)) {
>>>> + rel_backing = g_strdup(backing);
>>>> + goto free_and_exit;
>>>> + }
>>>
>>> This is a weaker form of path_is_absolute(), right? It only returns
>>> backing if the path is absolute and canonical. I don't think we really
>>> need the latter condition.
>>>
>>
>> Agree, and I would go further and say we don't need to return if it is
>> absolute. Just let the function do as its name implies, and always
>> return a relative path (except on error, as suggested below).
>
> Yes, I think that makes sense.
>
>>>> + break;
>>>> + } else if (strlen(src_tmp) <= 1) {
>>>> + break;
>>>> + }
>>>> + src_dir = dirname(src_tmp);
>>>> + g_strlcpy(src_tmp, src_dir, strlen(src));
>>>
>>> Same as above.
>>>
>>
>> OK. Although, if src_tmp starts out as canonical (src), dirname should
>> never return something longer than src.
>
> Hm, unexpected answer... Just to make sure we're not talking past each
> other: I meant the undefined behaviour because src_dir and src_tmp can
> overlap.
>
OK, we were talking past each other. I thought you meant if g_strlcpy()
truncated, due to strlen(src_dir) being longer than strlen(src).
> Kevin
