After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 689813 - Missing call to va_end in gupnp-service-proxy.c
Missing call to va_end in gupnp-service-proxy.c
Status: RESOLVED FIXED
Product: GUPnP
Classification: Other
Component: gupnp
0.19.x
Other Linux
: Normal normal
: ---
Assigned To: Krzesimir Nowak
GUPnP Maintainers
ivi
Depends on:
Blocks:
 
 
Reported: 2012-12-06 21:15 UTC by Mark Ryan
Modified: 2013-03-01 08:19 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix valist issue by routing _valist functions to its _hash counterparts. (15.31 KB, patch)
2013-02-27 16:38 UTC, Krzesimir Nowak
none Details | Review
Fix valist issue by routing _valist functions to its _hash counterparts. v2 (16.66 KB, patch)
2013-02-28 08:42 UTC, Krzesimir Nowak
none Details | Review
Fix a typo in valist patch. (1.12 KB, patch)
2013-02-28 14:49 UTC, Krzesimir Nowak
none Details | Review

Description Mark Ryan 2012-12-06 21:15:23 UTC
This is the last of the Coverity bugs.  Coverity points out that G_VA_COPY is performed at line 891 of gupnp-service-proxy.c to make a copy of va_args.  The copy is stored in an instance of GUPnPServiceProxyAction.  As far as I can tell va_end is never called on this copy.  I'm afraid I don't have a patch for this yet but I think Coverity is right here, so I'm entering the bug.
Comment 1 Jens Georg 2012-12-09 20:26:19 UTC
it does call va_end on it, but only in the _send_action case, it's dangling in the _begin_action case. However, reading the manpage of va_*, I'm kindof sceptic about the whole way va_args are used here:

"Each invocation of va_start()/va_copy() must be matched by a corresponding invocation of va_end() in the same function."

I mean "same function" here. I usually avoid varargs all together, so I've no idea if this is correct or not.
Comment 2 Krzesimir Nowak 2013-02-27 16:38:04 UTC
Created attachment 237533 [details] [review]
Fix valist issue by routing _valist functions to its _hash counterparts.
Comment 3 Krzesimir Nowak 2013-02-28 08:42:19 UTC
Created attachment 237581 [details] [review]
Fix valist issue by routing _valist functions to its _hash counterparts. v2

I decided to leave hash table <-> va_list conversion routines as macros, because in some places I expect valist to be modified in place which might be not guaranteed if I pass valist to a function. It's +10 to ugliness, but we already got +100 to it for var args anyway.
Comment 4 Krzesimir Nowak 2013-02-28 14:49:01 UTC
Created attachment 237600 [details] [review]
Fix a typo in valist patch.

I pushed those patches by accident. Here I am attaching a patch fixing a small issue I found in my patch.