[libvirt] [PATCH v2 1/4] util: Rewrite virGetUserDirectory() using g_get_home_dir()

[Cole Robinson]

On 12/18/19 11:12 AM, Fabiano Fidêncio wrote:
> On Wed, Dec 18, 2019 at 5:08 PM Daniel P. Berrangé <berrange at redhat.com> wrote:
>>
>> On Wed, Dec 18, 2019 at 10:53:41AM -0500, Cole Robinson wrote:
>>> On 12/18/19 5:34 AM, Fabiano Fidêncio wrote:
>>>> On Wed, Dec 18, 2019 at 11:19 AM Marc Hartmayer <mhartmay at linux.ibm.com> wrote:
>>>>>
>>>>> On Tue, Dec 17, 2019 at 05:41 PM +0100, Fabiano Fidêncio <fidencio at redhat.com> wrote:
>>>>>> Signed-off-by: Fabiano Fidêncio <fidencio at redhat.com>
>>>>>> ---
>>>>>>  src/util/virutil.c | 2 +-
>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/src/util/virutil.c b/src/util/virutil.c
>>>>>> index ed1f696e37..8c255abd7f 100644
>>>>>> --- a/src/util/virutil.c
>>>>>> +++ b/src/util/virutil.c
>>>>>> @@ -582,7 +582,7 @@ virGetHostnameQuiet(void)
>>>>>>  char *
>>>>>>  virGetUserDirectory(void)
>>>>>>  {
>>>>>> -    return virGetUserDirectoryByUID(geteuid());
>>>>>> +    return g_strdup_printf("%s/libvirt", g_get_home_dir());
>>>>>
>>>>>
>>>>> I would suggest to use 'g_build_path' instead of 'g_strdup_printf'.
>>>>> E.g.
>>>>>
>>>>> g_build_path(G_DIR_SEPARATOR_S, g_get_home_dir(), "libvirt");
>>>>
>>>
>>> Good catch
>>>
>>> There's also g_build_filename which sounds simpler. Any reason for one
>>> over the other?
>>
>> Based on the docs g_build_filename looks better, as it keeps
>> '/' vs '\' consistent on windows.  Aside from that, their
>> internal impl is basically the same.
> 
> Okay, I'll use g_build_filename() then.
> I wonder whether consistently using g_build_filename() wherever it's
> possible should be added to the BiteSizedTasks as well.

Certainly in places where we have windows specific code.

I feel like we must get path separator stuff wrong already in code that
is compiled for windows (netclient at least), but maybe there's some
gnulib magic that is handling path conversion for us?

- Cole