On Mon, Aug 13, 2018 at 10:23:02PM +0530, Sukrit Bhatnagar wrote:
On Mon, 13 Aug 2018 at 18:10, Erik Skultety <eskultet@redhat.com> wrote:
On Thu, Aug 09, 2018 at 09:42:12AM +0530, Sukrit Bhatnagar wrote:
By making use of GNU C's cleanup attribute handled by the VIR_AUTOPTR macro for declaring aggregate pointer variables, majority of the calls to *Free functions can be dropped, which in turn leads to getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/util/virnetlink.c | 72 ++++++++++++++++++++------------------------------- 1 file changed, 28 insertions(+), 44 deletions(-)
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index fcdc09d..66e80e2 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -297,15 +297,16 @@ int virNetlinkCommand(struct nl_msg *nl_msg, uint32_t src_pid, uint32_t dst_pid, unsigned int protocol, unsigned int groups) { - int ret = -1; struct sockaddr_nl nladdr = { .nl_family = AF_NETLINK, .nl_pid = dst_pid, .nl_groups = 0, }; struct pollfd fds[1]; - VIR_AUTOPTR(virNetlinkHandle) nlhandle = NULL; int len = 0; + VIR_AUTOPTR(virNetlinkHandle) nlhandle = NULL;
unjustified code movement...
+ + *respbuflen = 0;
unnecessary initialization..
We need *respbuflen to be 0 if -1 is returned. So if this initialization is removed, we need to add `*respbuflen = 0;` in the cleanup section below.
Why exactly do we need it? If -1 is to be returned, the caller should not be touching the pointers afterwards, if they are, it's a bug in the caller. Quick grepping on usage of virNetlinkCommand didn't show anything suspicious - we always either return immediately or jump to cleanup for that matter. Erik