PNM bugfix doesn't help

Nick Lamb (njl98r@ecs.soton.ac.uk)
Wed, 9 Dec 1998 19:33:22 +0000 (GMT)

On Wed, 9 Dec 1998 becka@rz.uni-duesseldorf.de wrote:

> If it is broken, it is probably largely my fault.

Good, I'm talking to the right guy.

> In what branch does that happen ?
>
> The sane-read request branches, if the "modifying" options are set.
> If neither grayify nor three-pass-emulation are set, all should be well:

Yes, I have neither grayify NOR three-pass-emulation set, I am using
the default settings and doing a preview scan.

> else
> /* Suck in as much of the file as possible, since it's already in the
> correct format. */
> len = fread (data, 1, max_length, infile);

Well, the code you have here is totally different to the code in the
SANE 1.0 source tarball I have. Sigh...

/* Suck in as much of the file as possible, since it's already in the
correct format. */
char *buf;
int doread = parms.bytes_per_line;
if (read_lines >= bry)
return SANE_STATUS_EOF;
buf = alloca(doread);
len = fread (buf, 1, doread, infile);

> Now let's have a look at the other branch ...

[snipped, I'm sure both cases are broken, because the PNM backend is
totally borked, and I'd like to see the author fix it, or rewrite it]

> - len = fread (q, 1, rgblength - rgbleftover[0], infile);
> + len = fread (q, 1, 3*max_length - rgbleftover[0], infile);
>
> Could you test/confirm that ? Otherwise we'd need to look for a "real" fix.

Tested, doesn't fix it (because we're in the other branch by this point)
Can't tell if it helps when Grayify/Three-pass are on, because they always
return total gibberish anyway. Make the easy case work first?

> > If the PNM device has more data to send, it just writes past the end
> > of the buffer,
>
> Hey - it's not that bad. Just some overly clever code, which misbehaves
> when used in a somewhat unusual way. Who didn't make such a mistake once
> upon a time ?

The SANE standard says I can pass any value for max_length, the PNM
backend assumes I will always pass >= sizeof(one scanline), which is
a totally bogus assumption. I notice on the list that at least one
other backend makes such bogus assumptions.

Perhaps I should write a test frontend, that behaves badly to crash
wrong-headed backends. If I did this would the SANE developers agree to
not allow distribution of backends that can't pass the test?
[They are non-conforming by SANE standard]

Alternatively the standard could be revised to say "Backends reserve
the right to copy any amount of data into the buffer, even if this
causes overflows or the end of world civilisation, so there."

Nick.

--
Source code, list archive, and docs: http://www.mostang.com/sane/
To unsubscribe: echo unsubscribe sane-devel | mail majordomo@mostang.com