help-gnu-utils
[Top][All Lists]
Advanced

[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.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]