[libvirt] [PATCH 0/2] virCommand: Two trivial fixes
Although not pushed, so review appreciated. Michal Privoznik (2): virCommandPassFD: Give name to flags virCommandFDIsSet: Update documentation src/util/vircommand.c | 13 ++++++------- src/util/vircommand.h | 4 ++-- 2 files changed, 8 insertions(+), 9 deletions(-) -- 2.16.1
The flags passed to virCommandPassFD() are unnamed and documentation to this function doesn't list them either. Give them name and mention it in documentation to functions using them. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/vircommand.c | 8 ++++---- src/util/vircommand.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 17cbdab7b7..8a319069fd 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -173,9 +173,9 @@ virCommandFDIsSet(virCommandPtr cmd, /* * virCommandFDSet: - * @fd: FD to be put into @set - * @set: the set - * @set_size: actual size of @set + * @cmd: pointer to virCommand + * @fd: file descriptor to pass + * @flags: extra flags; binary-OR of virCommandPassFDFlags * * This is practically generalized implementation * of FD_SET() as we do not want to be limited @@ -976,7 +976,7 @@ virCommandNewVAList(const char *binary, va_list list) * virCommandPassFD: * @cmd: the command to modify * @fd: fd to reassign to the child - * @flags: the flags + * @flags: extra flags; binary-OR of virCommandPassFDFlags * * Transfer the specified file descriptor to the child, instead * of closing it on exec. @fd must not be one of the three diff --git a/src/util/vircommand.h b/src/util/vircommand.h index d59278cf5f..883e212959 100644 --- a/src/util/vircommand.h +++ b/src/util/vircommand.h @@ -51,10 +51,10 @@ virCommandPtr virCommandNewVAList(const char *binary, va_list list) * delayed until the Run/RunAsync methods */ -enum { +typedef enum { /* Close the FD in the parent */ VIR_COMMAND_PASS_FD_CLOSE_PARENT = (1 << 0), -}; +} virCommandPassFDFlags; void virCommandPassFD(virCommandPtr cmd, int fd, -- 2.16.1
On Wed, Mar 21, 2018 at 05:28:33PM +0100, Michal Privoznik wrote:
The flags passed to virCommandPassFD() are unnamed and documentation to this function doesn't list them either. Give them name and mention it in documentation to functions using them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/vircommand.c | 8 ++++---- src/util/vircommand.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 17cbdab7b7..8a319069fd 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -173,9 +173,9 @@ virCommandFDIsSet(virCommandPtr cmd,
/* * virCommandFDSet: - * @fd: FD to be put into @set - * @set: the set - * @set_size: actual size of @set + * @cmd: pointer to virCommand + * @fd: file descriptor to pass + * @flags: extra flags; binary-OR of virCommandPassFDFlags * * This is practically generalized implementation * of FD_SET() as we do not want to be limited @@ -976,7 +976,7 @@ virCommandNewVAList(const char *binary, va_list list) * virCommandPassFD: * @cmd: the command to modify * @fd: fd to reassign to the child - * @flags: the flags + * @flags: extra flags; binary-OR of virCommandPassFDFlags * * Transfer the specified file descriptor to the child, instead * of closing it on exec. @fd must not be one of the three diff --git a/src/util/vircommand.h b/src/util/vircommand.h index d59278cf5f..883e212959 100644 --- a/src/util/vircommand.h +++ b/src/util/vircommand.h @@ -51,10 +51,10 @@ virCommandPtr virCommandNewVAList(const char *binary, va_list list) * delayed until the Run/RunAsync methods */
-enum { +typedef enum { /* Close the FD in the parent */ VIR_COMMAND_PASS_FD_CLOSE_PARENT = (1 << 0), -}; +} virCommandPassFDFlags;
I wonder how many more enums we have without typedefs. Perhaps worth enforcing that all enums have typedefs, though not sure how easy we can detect it with simple syntax-check rules - might need a full perl script. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
The set of arguments was changed a long time ago (040d9963420 which dates back to July 2013) but the corresponding documentation was not updated. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/vircommand.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 8a319069fd..6dab105f56 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -147,9 +147,8 @@ static int dryRunStatus; /* * virCommandFDIsSet: - * @fd: FD to test - * @set: the set - * @set_size: actual size of @set + * @cmd: pointer to virCommand + * @fd: file descriptor to query * * Check if FD is already in @set or not. * -- 2.16.1
On Wed, Mar 21, 2018 at 05:28:34PM +0100, Michal Privoznik wrote:
The set of arguments was changed a long time ago (040d9963420 which dates back to July 2013) but the corresponding documentation was not updated.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/vircommand.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 8a319069fd..6dab105f56 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -147,9 +147,8 @@ static int dryRunStatus;
/* * virCommandFDIsSet: - * @fd: FD to test - * @set: the set - * @set_size: actual size of @set + * @cmd: pointer to virCommand + * @fd: file descriptor to query * * Check if FD is already in @set or not. *
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 03/21/2018 12:28 PM, Michal Privoznik wrote:
Although not pushed, so review appreciated.
Michal Privoznik (2): virCommandPassFD: Give name to flags virCommandFDIsSet: Update documentation
src/util/vircommand.c | 13 ++++++------- src/util/vircommand.h | 4 ++-- 2 files changed, 8 insertions(+), 9 deletions(-)
ACK series. Reviewed-by: Laine Stump <laine@laine.org>
participants (3)
-
Daniel P. Berrangé -
Laine Stump -
Michal Privoznik