search

resurrecting-open-source-projects / scrot

SCReenshOT - command line screen capture utility
Other
495 stars 49 forks source link

Remove unnecessary string duplication #288

Closed N-R-K closed 1 year ago

N-R-K commented 1 year ago

There are many cases in scrot where strings are unnecessarily duplicated. The goal of this patchset is to remove these.

Most of these strings also were never free-ed, so this also technically fixes some memory leaks (although in practice this isn't that big of an issue since scrot is a "one shot" program, not a long running process).

A couple other cases worth investigating:

src/note.c:287:    return strndup(begin, length);
src/options.c:617:    opt.note = estrdup(optarg);

Also note: opt.lineMode is handled in #282.

N-R-K commented 1 year ago

This PR is accumulating some unrelated commits as well. So I think I'll just mark this ready for review for now - the other two instances can be investigated and fixed in another PR.

N-R-K commented 1 year ago

@guijan A review would be useful.

(For some reason github won't let me use the "request review" feature to request a review from you).

guijan commented 1 year ago

Probably because I'm not a collaborator. Will have a look tomorrow.

guijan commented 1 year ago

Incredible how in master opt.exec gets dup()'d for no reason, then the imPrintf() call in scrotExecapp() dup()s it again, and then both leak... Scrot is one of those code bases with about as many bugs as lines of code.

This touches some of the code I'm least familiar with, namely X programming, so I can't vouch for the PR, but it looks fine.

N-R-K commented 1 year ago

Rebased to solve conflict. Only other change is removing the assert in scrotSelectionGetLineColor.