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

Fix memleaks in http.c and encode.c #322

Closed
wants to merge 1 commit into from
Closed

Fix memleaks in http.c and encode.c #322

wants to merge 1 commit into from

Conversation

rikylescak
Copy link
Contributor

Hi,
there are two memory leaks found by valgrind in cups:

=25357== 41 bytes in 2 blocks are definitely lost in loss record 678 of 1,250
==25357== at 0x484186F: malloc (vg_replace_malloc.c:381)
==25357== by 0x4DB7FBE: strdup (strdup.c:42)
==25357== by 0x4CB01EF: http_add_field (http.c:3676)
==25357== by 0x4CB0E11: _httpUpdate (http.c:2891)
==25357== by 0x4CB0EBA: UnknownInlinedFun (http.c:2948)
==25357== by 0x4CB0EBA: httpUpdate (http.c:2918)
==25357== by 0x4CB25A8: http_tls_upgrade (http.c:4620)
==25357== by 0x4CC7A07: cupsGetResponse (request.c:438)
==25357== by 0x4CCBD4A: cupsDoIORequest (request.c:232)
==25357== by 0x125875: browse_poll_cancel_subscription (cups-browsed.c:11173)
==25357== by 0x11339F: main (cups-browsed.c:12784)

==25357== 480 (72 direct, 408 indirect) bytes in 1 blocks are definitely lost in loss record 1,204 of 1,250
==25357== at 0x4846464: calloc (vg_replace_malloc.c:1328)
==25357== by 0x4CB7581: ippNew (ipp.c:2622)
==25357== by 0x4CA5533: _cupsEncodeOption (encode.c:644)
==25357== by 0x4CA67D7: UnknownInlinedFun (encode.c:851)
==25357== by 0x4CA67D7: cupsEncodeOptions2 (encode.c:725)
==25357== by 0x122C0E: update_cups_queues (cups-browsed.c:8813)
==25357== by 0x4B0E980: g_timeout_dispatch (gmain.c:4933)
==25357== by 0x4B0E12F: UnknownInlinedFun (gmain.c:3381)
==25357== by 0x4B0E12F: g_main_context_dispatch (gmain.c:4099)
==25357== by 0x4B63207: g_main_context_iterate.constprop.0 (gmain.c:4175)
==25357== by 0x4B0D852: g_main_loop_run (gmain.c:4373)
==25357== by 0x11324C: main (cups-browsed.c:12747)

@zdohnal
Copy link
Member

zdohnal commented Jan 11, 2022

Hi @rikylescak ,

thank you for the PR!

Freeing the IPP structure collection looks fine here, use counter will prevent the complete free of the pointer and ippDelete() calls happen after ippSetCollection() calls on other places as well.
Memory allocation in _httpUpdate() happens when there is a known field in HTTP header during upgrading the connection - if httpClearFields() doesn't happen, memcpy() will rewrite the fields during restoring original values, so we lose the pointer.

LGTM, I'll start the test suite.

@zdohnal zdohnal self-assigned this Jan 11, 2022
@zdohnal zdohnal added bug Something isn't working priority-medium labels Jan 11, 2022
@zdohnal zdohnal added this to the v2.4.1 milestone Jan 11, 2022
Copy link
Member

@michaelrsweet michaelrsweet left a comment

Choose a reason for hiding this comment

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

LGTM

@zdohnal
Copy link
Member

zdohnal commented Jan 12, 2022

Merged as commit d992418 , thank you for the contribution!

@zdohnal zdohnal closed this Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority-medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants