resurrecting-open-source-projects / scrot

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

don't unnecessarily pass image to imPrintf #335

Closed N-R-K closed 1 year ago

N-R-K commented 1 year ago

there's no reason for imPrintf to set imlib2's image context. and it was redundant anyways, in all call sites, the proper image was already in context.

also remove image from scrotExecApp() as well, since it's now unused there too.

daltomi commented 1 year ago

Try: scrot -o "test.png" -t 10 -e 'echo bytes:$p'

log ***** Imlib2 Developer Warning ***** : This program is calling the Imlib call: imlib_image_get_width(); With the parameter: image being NULL. Please fix your program. ***** Imlib2 Developer Warning ***** : This program is calling the Imlib call: imlib_image_get_height(); With the parameter: image being NULL. Please fix your program. bytes:0
N-R-K commented 1 year ago

Good catch, thanks! The following diff works:

diff ```diff errx(EXIT_FAILURE, "Saving thumbnail %s failed", filenameThumb); } } + imlib_context_set_image(image); if (opt.exec) scrotExecApp(tm, filenameIM, filenameThumb); - imlib_context_set_image(image); imlib_free_image_and_decache(); free(filenameIM); ```

But maybe the current code is okay too since the imlib_context_set_image inside imPrintf isn't redundant. Thinking about closing the PR and keep things as is.

N-R-K commented 1 year ago

Closing this, having the context set closer to usage is less error prone (even if redundant in some cases).