On 10/08/12 19:57, Eric Blake wrote:
On 10/08/2012 11:44 AM, Marcelo Cerri wrote:
This comment is actually wrong... It should be:
/* Search in the password database for a user id that matches the user name
* `name`. Returns 0 on success, -1 on failure or 1 if name cannot be found.
Are you ok with this kind of return?
For a helper function, yes, that works.
Wondering aloud: as long as you are writing a helper function, does it
make sense to try and merge user and group parsing into a single
function, where you pass a function pointer (which will be either
getpwnam_r or getgrnam_r), or do we run into too many issues with
properly wording error messages?
I was wondering this myself when reviewing the original patchset. The
problem with merging those is different type of the arguments ("struct
passwd" vs "struct group") for those functions. I evaluated it as it's
not worth spending the effort of trying to combine them.
When rc is non-zero, we hit an error, and should use
virReportSystemError() to expose the errno value that explains the
failure. This part of the patch is wrong, as you have a regression in
the loss of a valid error message to the log; also, when rc is non-zero,
we should return -1.
The other alternative is to do NO error reporting in the helper function
(just VIR_DEBUG), return -errno on failure, 0 on success, and 1 on
lookup, and have the caller do the appropriate error reporting based on
the return value, where the caller then knows whether it was "user" or
"group" that failed.
This idea also seems reasonable (although we probably can't combine them).
Peter
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list