[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