[Xcb] [PATCH] util-cursor: Fix minor memleak and fix the RENDER version check

Josh Triplett josh at joshtriplett.org
Thu Sep 19 11:37:30 PDT 2013


On Thu, Sep 19, 2013 at 08:10:51PM +0200, Uli Schlachter wrote:
> On 19.09.2013 19:31, Josh Triplett wrote:
> > On Thu, Sep 19, 2013 at 07:09:01PM +0200, Uli Schlachter wrote:
> >> On 19.09.2013 18:47, Bart Massey wrote:
> >>> I'm confused about the first patch. Shouldn't you be checking whether
> >>> the resource string is null before freeing it, rather than freeing it
> >>> unconditionally?
> >>
> >> free(NULL) is fine. First result for "free null" on Google is:
> >>
> >> http://stackoverflow.com/questions/1938735/does-freeptr-where-ptr-is-null-corrupt-memory
> >>
> >> Or, quoting free(3) (the part about free):
> >>
> >>   If ptr is NULL, no operation is performed.
> >>
> >> Really, everyone who writes something like if (foo != NULL) free(foo); should be
> >> taught this.
> >>
> >> According to other Google results, even c89 says this already.
> > 
> > And generalizing, any type of memory reclamation function should follow
> > that pattern: if you write a function foo_free or foo_destroy or
> > similar, make it accept NULL and no-op, rather than making your callers
> > do that.
> 
> Uhm, should I turn the attached diff into a proper commit? :-P

> diff --git a/src/xcb_conn.c b/src/xcb_conn.c
> index fd9b4d8..dff96c6 100644
> --- a/src/xcb_conn.c
> +++ b/src/xcb_conn.c
> @@ -308,7 +308,7 @@ xcb_connection_t *xcb_connect_to_fd(int fd, xcb_auth_info_t *auth_info)
>  
>  void xcb_disconnect(xcb_connection_t *c)
>  {
> -    if(c->has_error)
> +    if(!c || c->has_error)
>          return;
>  
>      free(c->setup);

Given that xcb_disconnect frees the connection: yes, please do, and
update the documentation accordingly to say that NULL is acceptable.

- Josh Triplett


More information about the Xcb mailing list