Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

more X memory macro fixes #605

Merged

Conversation

sunweaver
Copy link
Member

Two commits cleaning up Xfree() vs XFree() vs free() and Xmalloc() vs. malloc().

Addressing #553. Required for 3.5.99.11 release.

@sunweaver sunweaver changed the title more x memory macro fixes more X memory macro fixes Dec 14, 2017
@sunweaver sunweaver requested a review from uli42 December 14, 2017 10:32
@sunweaver sunweaver added this to the 3.6.0.0 milestone Dec 14, 2017
@sunweaver
Copy link
Member Author

@uli42: here is your fix for #553. Please review.

Copy link
Member

@uli42 uli42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks ok. Generally this code uses far to many empty lines. We could probably save a huge amount of LOC here by removing them, removing the NULL checks around free and introducing FREE_SAFE plus some other stuff.

}

if (handler != NULL)
{
Xfree(handler);
free(handler);
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could remove the check for NULL altogether here and at numerous other places

@@ -4523,7 +4523,7 @@ static Bool _NXCollectInputFocusHandler(Display *dpy, xReply *rep, char *buf,

DeqAsyncHandler(dpy, state -> handler);

Xfree(state -> handler);
free(state -> handler);

state -> handler = NULL;
Copy link
Member

@uli42 uli42 Dec 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have introduced the FREE_SAFE macro to nxcomp. I suggest using it here, too. Would save many lines...

Xfree(NXImageCache[NXImageCacheSize - 1].image);
Xfree(NXImageCache[NXImageCacheSize - 1].md5);
free(NXImageCache[NXImageCacheSize - 1].image -> data);
free(NXImageCache[NXImageCacheSize - 1].image);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add a freeImage() function for that.

@sunweaver
Copy link
Member Author

@uli42: Thanks for your comments. I will file a separate issue (or more than one) for those comments and do the merge (and release of 3.5.99.11) now.

@Ionic
Copy link
Member

Ionic commented Dec 15, 2017

*SAFE_FREE

 They have been flawed ever since nxagent came up, as they were Xfree
 (non-capital f) always, but should have been XFree (capital F, defined
 in Xlibint.h and part of libNX_X11).
 .
 Probably this all should be free() all over the code (bearing in mind,
 that XFree() returns int, not void. But still...

 Fixes ArcticaProject#553.
@sunweaver sunweaver merged commit 5597f2e into ArcticaProject:3.6.x Dec 15, 2017
sunweaver added a commit that referenced this pull request Dec 15, 2017
Attributes GH PR #605: #605

Reviewed-by: Ulrich Sibiller <uli42@gmx.de> --  Thu, 14 Dec 2017 13:15:12 +0000
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants