[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] sharutils-4.15.2/src/uuencode.c
From: |
Stefan Kanthak |
Subject: |
Re: [PATCH] sharutils-4.15.2/src/uuencode.c |
Date: |
Sat, 18 Jun 2022 16:41:29 +0200 |
"Bruce Korb" <bkorb@gnu.org> wrote:
> 1. have you verified that the input buffer has the extra 2 bytes and
> that it's okay to scribble there?
There's no need to verify the first condition, you can proof it:
- sizeof(buf) is 15*3,
- rdct is at most sizeof(buf),
therefore (rdct + rdct % 3) is always within the buffer.
BTW: the following addition to my patch should make this more obvious:
@@ -164,1 +170,1
- if (rdct < 45)
+ if (rdct < sizeof(buf))
If (rdct < sizeof(buf)) then in[rdct] was not read, and
if (rdct < sizeof(buf) - 1) then in[rdct+1] was not read too, so they can
safely be set to '\0'; you can even clear buf before the fread() or add
memset(buf + rdct, 0, sizeof(buf) - rdct);
after the fread() to clear the tail of the input buffer not filled by the
fread()
> 2. have you run timing tests on the fully optimized code to see if it
> actually saves some microseconds?
No.
> Long, long ago, someone came up with this code, presumably after
> doing some analysis. I've never looked at the compiled output, so I
> can't answer. :)
The code is shorter, cleaner and not slower.
> 3. you do realize that there's almost nobody left on the planet using
> this, right?
Of course; nevertheless the clumsy handling of the last 1 or 2 input
characters should be removed: think of the innocent children that
might look into that code...
regards
Stefan
> On 6/18/22 05:01, Stefan Kanthak wrote:
>> Remove the almost duplicate code to process the last 1 or 2 input characters;
>> Access each input character only once, not twice;
>> Don't decrement 'in_len' AND increment 'in' inside the loop, one
>> loop counter is enough.