Possible leaks from clang scan, need investigation

                        box = create_label(node->mode, node->text[count], node->search_text, node->actions[count]);
../../../src/wofi.c:460:2: warning: Attempt to free released memory
../../../src/wofi.c:1262:3: warning: Potential leak of memory pointed to by 'surface_listener'
Assigned to
2 years ago
2 years ago
No labels applied.

~scoopta 2 years ago

Hmmmmm, sourcehut didn't send me any emails about either of your two bugs or your patch. I've been distracted playing games but I really don't like to wait this long for an initial reply. Apologies. free(node->search_text) is the only free that's ever done on that memory. I'm not entirely sure how clang is thinking that's a double free? Line 710 does strdup() to allocate that memory and the only time it's ever freed anywhere is line 486. These line numbers are from the current tip. There is some fun stuff happening in create_label() but all of it is done with a duplicate of that string and never the string itself. Surface listener is technically a leak. I malloc() the memory and don't hold the pointer anywhere in wofi so it's a leak in that sense but the compositor holds that pointer and will use it if the surface needs to be reconfigured. If that were to be stack memory then a segfault would occur. I could fix that by holding the pointer globally but I don't see a need other than to fix this error so I'm not sure it's worth it. Hopefully I didn't miss anything.

~scoopta 2 years ago

On further examination it looks like the double free is probably due to lines 334 and 349 in create_label(). Clang is probably not aware that the only time the free on 349 happens is if 334 dups the string. One cannot happen without the other but if somehow they were to happen individually then that could cause that double free and static analysis will be missing that info.

~scoopta 2 years ago

Anything further with this? If not I'll close it. Also I got you confused with someone else when I said:

sourcehut didn't send me any emails about either of your two bugs or your patch

too many usernames that start with t and my brain skipped the rest lol.

~scoopta REPORTED WONT_FIX 2 years ago

Register here or Log in to comment, or comment via email.