[sane-devel] Fixing sequencing issues (was Re: Proposed Upgrade (pu) branches on GitLab)

[Johannes Meixner]

Hello,

an addendum only for the fun of it that is not meant
as starting point for a discussion about coding style:

On Sep 28 21:07 m. allan noah wrote (excerpt):
> On Mon, Sep 28, 2015 at 7:04 AM, Johannes Meixner <jsmeix at suse.de> wrote:
>> In sane-backends-git20150928 in backend/microtek2.c
>> there is line 7456:
>>
>>         ms->buf.current_src = ++ms->buf.current_src % 2;
>>
>> My "decent educated guess" is that
>>
>>         ++ms->buf.current_src;
>>         ms->buf.current_src = ms->buf.current_src % 2;
>>
>> is meant.
>
> I'd guess it is just trying to swap 0 and 1, since all it is used for
> is an index into ms->buf.src_buffer, which is defined as:
> uint8_t *src_buffer[2];     /* two buffers because of CCD gap */
>
> So all the code should do is:
>
> ms->buf.current_src = ! ms->buf.current_src;
>

A perfect example for a "source code extremist" (TM Olaf Meeuwissen)
point of view ;-)

I think when the actual meaning is to toggle between
using src_buffer[0] and src_buffer[1] then the toggling
should be made obvious by explicit coding.

For the fun of it here an example what I mean:
--------------------------------------------------------------------------
// array[size > 2] cyclic access versus array[2] access via toggling:

#include <stdio.h>
#include <stdlib.h>

#define array_size 3

int
main (int argc, char *argv[])
{
   int i;

   int array[array_size];
   int array_index;

   for (array_index = 0; array_index < array_size; ++array_index)
   { array[array_index] = array_index * 2;
   }

   array_index = 0;
   for (i = 0; i < 6; ++i)
   { fprintf (stdout, "array[%i]=%i\n", array_index, array[array_index]);
     // next array element and wrap around:
     array_index = (array_index + 1) % array_size;
   }

   char toggle[] = { 'a', 'b' };
   int toggle_state = 0;

   for (i = 1; i <= 6; ++i)
   { fprintf (stdout, "toggle[%i]=%c\n", toggle_state, toggle[toggle_state]);
     // Bad because fragile dependency on i (fails for i=0..5)
     // and obscure constant 2:
     //   toggle_state = i % 2;
     // Better but obscure not operation insted of explicit value setting:
     //   toggle_state = ! toggle_state;
     // Explicit toggling "if it is 0 make it 1 otherwise make it 0":
     toggle_state = (toggle_state == 0) ? 1 : 0;
   }

   return EXIT_SUCCESS;

}
--------------------------------------------------------------------------

So from my "source code extremist" point of view
explicite coding should be:

ms->buf.current_src = (ms->buf.current_src == 0) ? 1 : 0;

Again: This is only for the fun of it and not intended
to have microtek2.c changed again.


Kind Regards
Johannes Meixner
-- 
SUSE LINUX GmbH - GF: Felix Imendoerffer, Jane Smithard,
Graham Norton - HRB 21284 (AG Nuernberg)