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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/18/19 11:12 AM, Fabiano Fidêncio wrote:
> On Wed, Dec 18, 2019 at 5:08 PM Daniel P. Berrangé <berrange@xxxxxxxxxx> 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@xxxxxxxxxxxxx> wrote:
>>>>>
>>>>> On Tue, Dec 17, 2019 at 05:41 PM +0100, Fabiano Fidêncio <fidencio@xxxxxxxxxx> wrote:
>>>>>> Signed-off-by: Fabiano Fidêncio <fidencio@xxxxxxxxxx>
>>>>>> ---
>>>>>>  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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux