Re: [libvirt] [PATCH] Return more error output if policykit auth fails.
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