Re: [PATCH v3 08/34] Adapt to VIR_STRDUP and VIR_STRNDUP in src/locking/*

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05/03/2013 08:53 AM, Michal Privoznik wrote:
> ---
>  src/locking/lock_daemon.c          | 30 +++++++++++-----------------
>  src/locking/lock_daemon_config.c   | 12 +++++------
>  src/locking/lock_daemon_dispatch.c |  6 ++----
>  src/locking/lock_driver_lockd.c    | 41 ++++++++++++++++----------------------
>  src/locking/lock_driver_sanlock.c  | 12 ++++-------
>  src/locking/lock_manager.c         |  4 +---
>  6 files changed, 41 insertions(+), 64 deletions(-)
> 
> diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c

> @@ -1221,13 +1217,13 @@ int main(int argc, char **argv) {
>  
>          case 'p':
>              VIR_FREE(pid_file);
> -            if (!(pid_file = strdup(optarg)))
> +            if (VIR_STRDUP(pid_file, optarg) < 0)
>                  exit(EXIT_FAILURE);

Hmm, for the similar patch to libvirtd earlier in the series, you used
VIR_STRDUP_QUIET and an explicit VIR_WARN when inside main() on the
grounds that it's too early to rely on normal error handling being up.
Do we need to do that here as well?

>              break;
>  
>          case 'f':
>              VIR_FREE(remote_config_file);
> -            if (!(remote_config_file = strdup(optarg)))
> +            if (VIR_STRDUP(remote_config_file, optarg) < 0)
>                  exit(EXIT_FAILURE);

Again, a case where immediately calling exit() may eat the error
message, compared to VIR_STRDUP_QUIET/VIR_WARN.

ACK once you consider that point.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]