Andrea Bolognani writes:
> On Mon, 2019-07-29 at 18:11 +0100, Daniel P. Berrangé wrote:
>> util: storage identity attrs as virTypedParameter internally
>
>> - if (virStrToLong_i(userid, NULL, 10, &val) < 0)
>> + ret = virTypedParamsGetULLong(ident->params,
>> + ident->nparams,
>> + VIR_CONNECT_IDENTITY_OS_USER_ID,
>> + &val);
>> + if (ret < 0)
>> return -1;
>>
>> - *uid = (uid_t)val;
>> + if (ret == 1)
>> + *uid = (uid_t)val;
>
> In case Christophe is following along: this is one of the cases where
> libvirt functions don't follow the usual 0 means success, <0 means
> failure mantra.
Thanks ;-)
>
> [...]
>> int virIdentityGetOSProcessID(virIdentityPtr ident,
>> pid_t *pid)
>> {
>> - unsigned long long val;
>> - const char *processid;
>> + int ret;
>> + long long val;
>
> I still think we should be using ull for pids.
Curious why (I'm too lazy to look it up in earlier discussions)?
In general, giving a name to an int type is a good idea, isn't it?
>
>> - *pid = (pid_t)val;
>> + if (ret == 1)
>> + *pid = (gid_t)val;
>
> This should be
>
> *pid = (pid_t)val;
You made me look at that code again, and now I'm confused as to why it's
OK to leave garbage in *pid if we fail to find the corresponding typed
param. Previously, the function returned -1 in that case, to indicate
failure. Now, it returns 0, but does not update *uid. Is that intentional?
>
> [...]
>> +++ b/tests/viridentitytest.c
>> @@ -166,7 +109,8 @@ static int testIdentityGetSystem(const void *data)
>> goto cleanup;
>>
>> if (STRNEQ_NULLABLE(val, context)) {
>> - VIR_DEBUG("Unexpected SELinux context attribute");
>> + VIR_DEBUG("Unexpected SELinux context attribute '%s' != '%s'",
>> + val, context);
>> goto cleanup;
>> }
>
> This change also doesn't belong in this patch. You can put it in the
> same one as the other SELinux-related test suite fix, though.
>
> And since you're tweaking the message, I suggest something like
>
> VIR_DEBUG("Unexpected SELinux context: expected='%s' actual='%s'",
> context, val);
>
> for easier debugging.
>
> --
> Andrea Bolognani / Red Hat / Virtualization
--
Cheers,
Christophe de Dinechin (IRC c3d)
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
