[libvirt] [PATCH] Return more error output if policykit auth fails.

Cole Robinson crobinso at redhat.com
Fri Jan 27 22:03:43 UTC 2012


On 01/27/2012 04:06 PM, Eric Blake wrote:
> On 01/27/2012 01:44 PM, Cole Robinson wrote:
>> @@ -2537,15 +2538,20 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED,
>>  error:
>>      virCommandFree(cmd);
>>      VIR_FREE(ident);
>> -    VIR_FREE(pkout);
>>      virResetLastError();
>> +
>>      if (authdismissed) {
>>          virNetError(VIR_ERR_AUTH_CANCELLED, "%s",
>>                      _("authentication cancelled by user"));
>> +    } else if (pkout || pkerr) {
>> +        virNetError(VIR_ERR_AUTH_FAILED, "%s %s", pkerr, pkout);
> 
> This could dereference NULL (well, it won't on glibc, but "(null) error"
> or "out (null)" don't look any nicer).
> 
> Change it to:
> 
>     virNetError(VIR_ERR_AUTH_FAILED, "%s", pkerr ? pkerr : pkout);
> 
> and I'll call it good enough for ACK (that is, even if the app wrote to
> both stdout and stderr, I think that printing just stderr output in that
> case is probably reasonable).
> 

Hmm, I actually want to always include both outputs. I'm afraid that at some
future point pkcheck will add some stderr spew and we won't show the more
important stdout output. But there are already cases where stderr is more
important, so I think showing both is best.

I sent a second patch that should avoid the (null) issue.

Thanks,
COle




More information about the libvir-list mailing list