Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Plugins
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Rbutil git
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by fg - 2010-01-02
Last edited by fg - 2010-02-11

FS#10889 - Close leaked filehandles on plugin exit

This patch makes plugin.c check for leaked file handles, and closes them if needed.
It can of course be trivially changed to panic instead.

On arm, there’s a binsize cost of about 256 bytes.

Not actually tested yet.

Closed by  fg
2010-02-11 19:21
Reason for closing:  Accepted
Additional comments about closing:   Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407

committed as r24598

Admin
fg commented on 2010-01-02 19:15

The double close() check is reversed. I’ll fix it later

I like the patch. I’d like any problem found with leaked file descriptors or double close to be more visible than just a logf, for example using a splash so users are triggered more to report a problem.

Admin
fg commented on 2010-01-02 21:26

I agree that logf is too weak, but I’m not sure if a splash isn’t too annoying.

Admin
fg commented on 2010-01-02 21:31

This patch should fix the double close() check. Please review, bitwise logic seems to make me dizzy today…

New version, the check can be disabled by not defining HAVE_PLUGIN_CHECK_OPEN_CLOSE in config.h.
Also wipe out wrappers if not needed and add a yesno dialog to inform the user to report the problem.

Admin
fg commented on 2010-01-09 13:28

Some comments:

- I don’t think any source file should have LOGF_ENABLE by default.
- Unifying the logf and GUI message strings should save some binsize. Not sure if it’s worth it though.
- Shouldn’t the GUI messages be translated?

New version:
-add translation support
-remove LOGF_ENABLE

New version:
-reuse an existing translation instead of a new one and had a little text (it could be change to an error number for example)

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing