Bug 385894 - plasma-desktop-5.11.1/kcms/hardware/joystick/joydevice.cpp:188]: (error) Memory leak
Summary: plasma-desktop-5.11.1/kcms/hardware/joystick/joydevice.cpp:188]: (error) Memo...
Status: RESOLVED FIXED
Alias: None
Product: systemsettings
Classification: Applications
Component: kcm_joystick (show other bugs)
Version: 5.11.1
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Unassigned bugs mailing-list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-10-18 07:27 UTC by dcb314
Modified: 2021-03-17 01:46 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.21.3
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description dcb314 2017-10-18 07:27:01 UTC
Source code is

  if ( ::ioctl(fd, JSIOCGCORR, oldCorr) == -1 )
  {
    ::close(fd);
    delete [] oldCorr;
    return JoyDevice::ERR_GET_CORR;
  }

  if (bt < 0) {
      return JoyDevice::ERR_GET_BUTTONS;
  }

Suggest add close and delete on second if statement.
Comment 1 Nate Graham 2017-10-18 15:58:58 UTC
Thanks for finding this bug! Please feel free to submit a patch on https://phabricator.kde.org/
Comment 2 dcb314 2017-10-18 19:53:51 UTC
Thanks for the offer, but I leave fixing bugs to the folks who know the code.
Comment 3 Nate Graham 2017-10-18 20:11:25 UTC
If seems like you do know the code! :) Or at least well enough to contribute. I'm rubbish at C++ but my paltry skill level seems to be good enough so far.

The worst thing that can happen is that you'll get a request for changes, and you'll learn something. Try it, you'll like it! This is a super welcoming community.
Comment 4 David Edmundson 2017-10-18 20:27:26 UTC
If that really is the code, just swap the two ifs() round
Comment 5 Justin Zobel 2021-03-12 07:23:47 UTC
Thanks for the report, is this memory leak still occurring or has the issue been fixed?

I've set this bug to NEEDSINFO. When you reply, please change the bug back to REPORTED so we know it's ready for investigation or RESOLVED/WORKSFORME if the issue is now resolved.
Comment 6 dcb314 2021-03-12 07:35:15 UTC
(In reply to Justin Zobel from comment #5)
> Thanks for the report, is this memory leak still occurring or has the issue
> been fixed?

I have no idea and no plans to find out. Submitting bug reports
that get ignored for more than three years looks pointless.

I'll be directing my bug finding efforts elsewhere in future.
You are on your own with this one.
Comment 7 Nate Graham 2021-03-12 20:10:53 UTC
We were all waiting on you, since you got the go-ahead to make the change. Open source is collaborative, with participation encouraged.
Comment 8 Bug Janitor Service 2021-03-12 20:13:09 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/plasma-desktop/-/merge_requests/381
Comment 9 Nate Graham 2021-03-12 20:30:28 UTC
Git commit 8a7afcb736cf857425938223f66bc84d5533f6b7 by Nate Graham.
Committed on 12/03/2021 at 20:11.
Pushed by cblack into branch 'master'.

[kcms/joystick] Fix minor leak

fd and oldcorr were not cleaned up before returning.
FIXED-IN: 5.21.3

M  +2    -0    kcms/joystick/joydevice.cpp

https://invent.kde.org/plasma/plasma-desktop/commit/8a7afcb736cf857425938223f66bc84d5533f6b7
Comment 10 Janet Blackquill 2021-03-12 20:31:01 UTC
Git commit 1913d9f5b4700c349ff0460ec0390673aac76f31 by Jan Blackquill, on behalf of Nate Graham.
Committed on 12/03/2021 at 20:30.
Pushed by cblack into branch 'Plasma/5.21'.

[kcms/joystick] Fix minor leak

fd and oldcorr were not cleaned up before returning.
FIXED-IN: 5.21.3
(cherry picked from commit 8a7afcb736cf857425938223f66bc84d5533f6b7)

M  +2    -0    kcms/hardware/joystick/joydevice.cpp

https://invent.kde.org/plasma/plasma-desktop/commit/1913d9f5b4700c349ff0460ec0390673aac76f31
Comment 11 Szczepan Hołyszewski 2021-03-16 17:00:48 UTC
> We were all waiting on you, since you got the go-ahead to make
> the change. Open source is collaborative, with participation
> encouraged.

It turns out that factually, what he got was more of a "go ahead, or else (the bug won't be fixed for another few years)", rather than a sincere go-ahead. And here's the thing: if you are the author/maintainer, gitwork is essentially YOUR JOB, and there is NO JUSTIFICATION for demanding that the user who did 100% of debugging also do the gitwork. There is NO JUSTIFICATION for not applying an obvious known fix FOR YEARS just because you want to make the point about "encouraging participation". A community that does that is not a "super welcoming" community, it is a "we can't be bothered to spend 5 minutes to formally wrap up a 2-LOC, single file result of what could have been hours or days of somebody else's debugging work" community. "Collaborative"? "I spend hours debugging, you spend 5 minutes to push my result through the right channels" is fair collaboration. "I spend hours debugging, you spend 15 minutes tutoring me interactively about the knowables and the unknowables of this project's submission process a.k.a. teach the man to fish" would also be fair collaboration. But "I spend hours debugging and you ask me to spend further hours (possibly spread over days) pushing the result through a territory unknown to me" is not fair collaboration.
Comment 12 Nate Graham 2021-03-17 01:46:09 UTC
OK, sorry you didn't have a good experience.