[libvirt] [PATCH 00/10] tools: remove vshStrdup (glib chronicles)

Ján Tomko posted 10 patches 4 years, 6 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/cover.1571438247.git.jtomko@redhat.com
tools/virsh-checkpoint.c     |  4 +--
tools/virsh-domain-monitor.c | 27 ++++++++++----------
tools/virsh-domain.c         | 38 ++++++++++++----------------
tools/virsh-nodedev.c        |  5 ++--
tools/virsh-pool.c           | 30 +++++++++++-----------
tools/virsh-snapshot.c       |  6 ++---
tools/virsh-volume.c         | 11 ++++-----
tools/virsh.c                |  9 +++----
tools/virt-admin.c           |  6 ++---
tools/vsh.c                  | 48 ++++++++++++++----------------------
tools/vsh.h                  |  4 ---
11 files changed, 81 insertions(+), 107 deletions(-)
[libvirt] [PATCH 00/10] tools: remove vshStrdup (glib chronicles)
Posted by Ján Tomko 4 years, 6 months ago
Ján Tomko (10):
  tools: cmdDomblkinfoGet: reindent parameters
  virsh: use g_strdup in cmdDomblkinfoGet
  virsh: use g_strdup in virshDomainGetEditMetadata
  virsh: getSignalNumber: rename variables
  virsh: getSignalNumber: use g_autofree
  virsh: getSignalNumber: use g_strdup
  tools: vshCommandArgvGetArg: one parameter per line
  tools: vshCommandArgvGetArg: prefer g_strdup
  tools: prefer g_strdup to vshStrdup
  tools: delete vshStrdup

 tools/virsh-checkpoint.c     |  4 +--
 tools/virsh-domain-monitor.c | 27 ++++++++++----------
 tools/virsh-domain.c         | 38 ++++++++++++----------------
 tools/virsh-nodedev.c        |  5 ++--
 tools/virsh-pool.c           | 30 +++++++++++-----------
 tools/virsh-snapshot.c       |  6 ++---
 tools/virsh-volume.c         | 11 ++++-----
 tools/virsh.c                |  9 +++----
 tools/virt-admin.c           |  6 ++---
 tools/vsh.c                  | 48 ++++++++++++++----------------------
 tools/vsh.h                  |  4 ---
 11 files changed, 81 insertions(+), 107 deletions(-)

-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/10] tools: remove vshStrdup (glib chronicles)
Posted by Daniel Henrique Barboza 4 years, 6 months ago

On 10/18/19 7:37 PM, Ján Tomko wrote:
> Ján Tomko (10):
>    tools: cmdDomblkinfoGet: reindent parameters
>    virsh: use g_strdup in cmdDomblkinfoGet
>    virsh: use g_strdup in virshDomainGetEditMetadata
>    virsh: getSignalNumber: rename variables
>    virsh: getSignalNumber: use g_autofree
>    virsh: getSignalNumber: use g_strdup
>    tools: vshCommandArgvGetArg: one parameter per line
>    tools: vshCommandArgvGetArg: prefer g_strdup
>    tools: prefer g_strdup to vshStrdup
>    tools: delete vshStrdup


All patches:

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>


Question: I see that you're pushing the glib chronicles recently. Do you 
have plans to do the virAsprintf -> g_strdup_printf conversion?

I am asking because I started this conversion yesterday, and the amount 
of calls to be converted is so massive that it's kind of funny. And it's 
not a work that I'll be able to full commit during the workday. So, if 
this conversion is under your radar and you want to get it done ASAP 
(meaning that you'll get it done faster than I'm capable of), let me 
know and I'll stand down from pursuing that.


Thanks,


DHB


> 
>   tools/virsh-checkpoint.c     |  4 +--
>   tools/virsh-domain-monitor.c | 27 ++++++++++----------
>   tools/virsh-domain.c         | 38 ++++++++++++----------------
>   tools/virsh-nodedev.c        |  5 ++--
>   tools/virsh-pool.c           | 30 +++++++++++-----------
>   tools/virsh-snapshot.c       |  6 ++---
>   tools/virsh-volume.c         | 11 ++++-----
>   tools/virsh.c                |  9 +++----
>   tools/virt-admin.c           |  6 ++---
>   tools/vsh.c                  | 48 ++++++++++++++----------------------
>   tools/vsh.h                  |  4 ---
>   11 files changed, 81 insertions(+), 107 deletions(-)
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/10] tools: remove vshStrdup (glib chronicles)
Posted by Ján Tomko 4 years, 6 months ago
On Sat, Oct 19, 2019 at 07:21:15AM -0300, Daniel Henrique Barboza wrote:
>
>
>On 10/18/19 7:37 PM, Ján Tomko wrote:
>>Ján Tomko (10):
>>   tools: cmdDomblkinfoGet: reindent parameters
>>   virsh: use g_strdup in cmdDomblkinfoGet
>>   virsh: use g_strdup in virshDomainGetEditMetadata
>>   virsh: getSignalNumber: rename variables
>>   virsh: getSignalNumber: use g_autofree
>>   virsh: getSignalNumber: use g_strdup
>>   tools: vshCommandArgvGetArg: one parameter per line
>>   tools: vshCommandArgvGetArg: prefer g_strdup
>>   tools: prefer g_strdup to vshStrdup
>>   tools: delete vshStrdup
>
>
>All patches:
>
>Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>
>
>Question: I see that you're pushing the glib chronicles recently. Do 
>you have plans to do the virAsprintf -> g_strdup_printf conversion?
>
>I am asking because I started this conversion yesterday, and the 
>amount of calls to be converted is so massive that it's kind of funny. 
>And it's not a work that I'll be able to full commit during the 
>workday

mprivozn started working on that on Friday using Coccinelle, you need to
ask him. Chances are you already converted some usage that was not
simply caught by his spatch.

Personally I'm working on a branch that does VIR_STRDUP -> g_strdup.

>So, if this conversion is under your radar and you want to 

https://en.wiktionary.org/wiki/under_the_radar

Jano

>get it done ASAP (meaning that you'll get it done faster than I'm 
>capable of), let me know and I'll stand down from pursuing that.
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/10] tools: remove vshStrdup (glib chronicles)
Posted by Daniel Henrique Barboza 4 years, 6 months ago

On 10/19/19 12:55 PM, Ján Tomko wrote:
> On Sat, Oct 19, 2019 at 07:21:15AM -0300, Daniel Henrique Barboza wrote:
>>
>>
>> On 10/18/19 7:37 PM, Ján Tomko wrote:
>>> Ján Tomko (10):
>>>   tools: cmdDomblkinfoGet: reindent parameters
>>>   virsh: use g_strdup in cmdDomblkinfoGet
>>>   virsh: use g_strdup in virshDomainGetEditMetadata
>>>   virsh: getSignalNumber: rename variables
>>>   virsh: getSignalNumber: use g_autofree
>>>   virsh: getSignalNumber: use g_strdup
>>>   tools: vshCommandArgvGetArg: one parameter per line
>>>   tools: vshCommandArgvGetArg: prefer g_strdup
>>>   tools: prefer g_strdup to vshStrdup
>>>   tools: delete vshStrdup
>>
>>
>> All patches:
>>
>> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>>
>>
>> Question: I see that you're pushing the glib chronicles recently. Do 
>> you have plans to do the virAsprintf -> g_strdup_printf conversion?
>>
>> I am asking because I started this conversion yesterday, and the 
>> amount of calls to be converted is so massive that it's kind of funny. 
>> And it's not a work that I'll be able to full commit during the workday
> 
> mprivozn started working on that on Friday using Coccinelle, you need to
> ask him. Chances are you already converted some usage that was not
> simply caught by his spatch.


Nevermind then. I'll let Michal do that, then he can do it all at once
and we'll not duplicate efforts. Thanks for letting me know.


> 
> Personally I'm working on a branch that does VIR_STRDUP -> g_strdup.
> 
>> So, if this conversion is under your radar and you want to 
> 
> https://en.wiktionary.org/wiki/under_the_radar
> 


Owned. I meant 'on your radar' and wrote the opposite :P


DHB



> Jano
> 
>> get it done ASAP (meaning that you'll get it done faster than I'm 
>> capable of), let me know and I'll stand down from pursuing that.
>>

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

Re: [libvirt] [PATCH 00/10] tools: remove vshStrdup (glib chronicles)
Posted by Michal Privoznik 4 years, 6 months ago
On 10/19/19 5:55 PM, Ján Tomko wrote:
> On Sat, Oct 19, 2019 at 07:21:15AM -0300, Daniel Henrique Barboza wrote:
>>
>>
>> On 10/18/19 7:37 PM, Ján Tomko wrote:
>>> Ján Tomko (10):
>>>   tools: cmdDomblkinfoGet: reindent parameters
>>>   virsh: use g_strdup in cmdDomblkinfoGet
>>>   virsh: use g_strdup in virshDomainGetEditMetadata
>>>   virsh: getSignalNumber: rename variables
>>>   virsh: getSignalNumber: use g_autofree
>>>   virsh: getSignalNumber: use g_strdup
>>>   tools: vshCommandArgvGetArg: one parameter per line
>>>   tools: vshCommandArgvGetArg: prefer g_strdup
>>>   tools: prefer g_strdup to vshStrdup
>>>   tools: delete vshStrdup
>>
>>
>> All patches:
>>
>> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>>
>>
>> Question: I see that you're pushing the glib chronicles recently. Do 
>> you have plans to do the virAsprintf -> g_strdup_printf conversion?
>>
>> I am asking because I started this conversion yesterday, and the 
>> amount of calls to be converted is so massive that it's kind of funny. 
>> And it's not a work that I'll be able to full commit during the workday
> 
> mprivozn started working on that on Friday using Coccinelle, you need to
> ask him. Chances are you already converted some usage that was not
> simply caught by his spatch.

Indeed, I just can't decide whether to drop all checks related to 
virAsprintf() first and only after that do transition to 
g_strdup_printf() or do the transition first and then drop all checks 
related (since the glib function can't fail).

https://travis-ci.org/zippy2/libvirt/builds/600171599

https://travis-ci.org/zippy2/libvirt/builds/600597977

I will need to rebase both branches when Jano pushes his VIR_STRDUP 
branch though.

Michal

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