search

philpem / printer-driver-ptouch

P-Touch PT-series and QL-series printer driver for Linux (under CUPS)
GNU General Public License v2.0
97 stars 24 forks source link

Add support for PT-9700PC (with fixes) #5

Closed karlp closed 4 years ago

karlp commented 4 years ago

As discussed in #3 This supersedes PR #4

philpem commented 4 years ago

Per discussion on #4, I'm happy with this except for how the right_padding_bits calculation is done.

The root cause is an integer overflow caused by a negative number being cast as unsigned, when (left_spacing_px + cupsWidth + right_spacing_px) is greater than (bytesPerLine * 8).

A fix along these lines seems reasonable: https://github.com/trialinfo/ptouch-driver/commit/9f1152cd2fb234cd6f38b6b491a1dea226922a55

I'm also going to take your point about -Wconversion on board and open a separate issue to add that to the compiler flags. Clearly we need to do some clean-up. I have some other ideas for compiler warning flags we should have enabled, based on other project work in a similar area.

philpem commented 4 years ago

Merged. @karlp - many thanks for the bug report, your work debugging it, and the constructive suggestions for other code improvements :)

karlp commented 4 years ago

Hopefully never again, but you're very welcome, thank you for being so responsive (both of you) with this!

philpem commented 4 years ago

I got the patch description a slightly wrong, sorry. Here's a fixed version: trialinfo/ptouch-driver@d167ea9

It's fine, the commit description (aside from the first word) is accurate and it only changes one file.

karlp commented 4 years ago

Heh, I fixed the "interger" typo and didn't notice the "ptexplain" leader :)