[sane-devel] Cleanup and small fixes
Olaf Meeuwissen
paddy-hack at member.fsf.org
Sun Dec 20 08:55:20 UTC 2015
Hi again Volker,
Olaf Meeuwissen writes:
> Volker Diels-Grabsch writes:
>
>> Dear SANE developers,
>>
>> Volker Diels-Grabsch schrieb:
>>> I'm not sure which is the best way to contribute, so I'm providing
>>> this as a series of (mostly) independent patches to this mailing list.
>>> I could also open one or more tickets in the issue tracker, if that
>>> helps.
>>
>> Is there anyone who likes to review my work?
>
> I'll see if I can get myself to review your patches this weekend. If
> anyone else beats me to it, that's okay.
* 0001-Fix-typos-in-comments.patch.gz
There's about 100 other "annoying" `dont`'s you didn't fix ;-), but
there is nothing wrong with your patch.
* 0002-Bugfix-On-error-return-the-actual-error-code-in-sane.patch.gz
Nice catch.
* 0003-Fix-interface-of-helper-function-write_many.patch.gz
I'm all in favour of const-correctness and less casts.
* 0004-Fix-scope-of-negation.patch.gz
Why not simply write: if (dev->model->is_sheetfed == SANE_FALSE)?
I've skipped your patch and pushed one that does the above.
* 0005-Introduce-md5_set_uint32.patch.gz
Hmm, fixing code that originated from glibc an aeon ago.
Maybe we should consider updating with more recent upstreams rather
than trying to patch up things ourselves.
Skipping this for now.
* 0006-Mark-internal-function-toupper_ascii-as-static.patch.gz
Clarifies intended scope and help finding dead code. Good.
* 0007-Ensure-that-sanei_thread_waitpid-always-has-a-define.patch.gz
I'm not sure on what the expected return value should be in case of
failure. The documentation in include/sane/sanei_thread.h is of no
help here.
On top of that, the implementation caters to at least three different
types of Sane_PID: int and two flavours of the implementation defined
pthread_t type.
Skipping this for now.
* 0008-Remove-dead-code-due-to-unused-variables.patch.gz
OK. The unused variables are not used in #ifdef'd code either so they
can safely be zapped.
* 0009-Add-dummy-code-snippets-to-ensure-that-no-translatio.patch.gz
If nothing of the file is used, it shouldn't be compiled in the first
place. I'll see if I can fix this as part of my autotool-reform
branch over at GitLab[1].
Skipping this for now.
* 0010-Merge-all-compatibility-macros-around-__func__-and-_.patch.gz
* 0011-Use-consistently-__func__-instead-of-__FUNCTION__.patch.gz
Thanks for cleaning this up!
* 0012-Change-GCC-mode-from-ISO-C90-to-ISO-C99.patch.gz
This is actually less controversial than you might think. It has been
discussed[2][3] after we released 1.0.25 and is on my very unofficial
milestone[4] for 1.0.26.
I don't think your patch is complete but it'll do for now.
>> What can I do to simplify reviewing for you? Should I post them
>> one-by-one to the issue tracker?
>
> That would at least prevent them from dropping off the radar.
[1] https://gitlab.com/sane-project/backends/tree/autotool-reform
[2] https://lists.alioth.debian.org/pipermail/sane-devel/2015-October/034002.html
[3] https://lists.alioth.debian.org/pipermail/sane-devel/2015-October/034019.html
[4] https://gitlab.com/sane-project/backends/milestones/1
Hope this helps,
--
Olaf Meeuwissen, LPIC-2 FSF Associate Member since 2004-01-27
Support Free Software Support the Free Software Foundation
https://my.fsf.org/donate https://my.fsf.org/join
GnuPG key: F84A2DD9/B3C0 2F47 EA19 64F4 9F13 F43E B8A4 A88A F84A 2DD9
More information about the sane-devel
mailing list