On 01/24/2018 07:48 AM, Marc Hartmayer wrote:
> On Wed, Jan 24, 2018 at 01:03 PM +0100, John Ferlan <jferlan@xxxxxxxxxx> wrote:
>> On 01/24/2018 05:12 AM, Marc Hartmayer wrote:
>>> On Fri, Jan 19, 2018 at 06:23 PM +0100, John Ferlan <jferlan@xxxxxxxxxx> wrote:
>>>> Once virNetServerProgramNew returns, the program objects have
>>>> either been added to the @srv or @srvAdm list increasing the
>>>> refcnt or there was an error. In either case, we can lower the
>>>> refcnt from virNetServerProgramNew and set the object to NULL
>>>> since it won't be used any more.
>>>>
>>>> The @srv or @srvAdm dispose function (virNetServerDispose) will
>>>> handle the Unref of each object during the virHashFree when the
>>>> object is removed from the hash table.
>>>>
>>>> Reviewed-by: Erik Skultety <eskultet@xxxxxxxxxx>
>>>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>>>> ---
>>>> daemon/libvirtd.c | 28 ++++++++++++++++++++--------
>>>> 1 file changed, 20 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
>>>> index 0afd1dd82..b47f875d9 100644
>>>> --- a/daemon/libvirtd.c
>>>> +++ b/daemon/libvirtd.c
>>>> @@ -1352,7 +1352,11 @@ int main(int argc, char **argv) {
>>>> ret = VIR_DAEMON_ERR_INIT;
>>>> goto cleanup;
>>>> }
>>>> - if (virNetServerAddProgram(srv, remoteProgram) < 0) {
>>>> +
>>>> + rc = virNetServerAddProgram(srv, remoteProgram);
>>>> + virObjectUnref(remoteProgram);
>>>> + remoteProgram = NULL;
>>>
>>>
>>> I’ve just looked at the code and all those variables are a global
>>> variables…
>>>
>>> libvirtd.c:
>>> virNetServerProgramPtr remoteProgram = NULL;
>>> virNetServerProgramPtr adminProgram = NULL;
>>> virNetServerProgramPtr qemuProgram = NULL;
>>> virNetServerProgramPtr lxcProgram = NULL;
>>>
>>> So this is not a good idea to set them to NULL… As this results in a
>>> segmentation fault
>>
>> Oh right... There really was no reason to set them to NULL since the
>> only time they're used is in the context they are used. If the
>> AddProgram was successful, then there would be 2 refs anyway. They're
>> not used later in the code, so I'll remove the = NULL after the Unref.
>
> Is it possible to get rid of these global variables? At least for
> adminProgram and lxcProgram this should be easily to achieve as they
> aren’t used anywhere else than in this function context.
I suppose so, not sure if that was within the context of what I was
doing. It'd have to be a separate "pre-patch" - in fact doable without
this series. I also know Dan's been working through adding admin
protocol support for logd/lockd in another series...
John
>
> libvirtd.h: (Only these variables are declared as extern)
> extern virNetServerProgramPtr remoteProgram;
> extern virNetServerProgramPtr qemuProgram;
>
>>
>> John
>>
>>>
>
> […snip…]
>
> --
> Beste Grüße / Kind regards
> Marc Hartmayer
>
> IBM Deutschland Research & Development GmbH
> Vorsitzende des Aufsichtsrats: Martina Koederitz
> Geschäftsführung: Dirk Wittkopp
> Sitz der Gesellschaft: Böblingen
> Registergericht: Amtsgericht Stuttgart, HRB 243294
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
