[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH] block: for a streaming job, fix relative ba
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [RFC PATCH] block: for a streaming job, fix relative base name arguments |
Date: |
Thu, 12 Apr 2012 15:41:38 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120329 Thunderbird/11.0.1 |
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.
Kevin