Various fixes related to string copying

[alejandro-colomar]

This consists of some patches from @eggert, and some patches of mine which have been more or less based on his suggestions. See the origin of the discussion at Debbugs: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1032393#159.

I didn't intend to remove strlcpy(3), since the bugs are independent of the use of strlcpy(3). I opted for fixing the bugs, and keeping strlcpy(3) where it's (IMO) the most appropriate call.

We might want to drop strlcpy(3), but I think that should be addressed separately, and after having fixed the bugs.

There's one patch from Paul that I didn't take or write an alternative, and that's because I didn't yet understand it. It's Avoid silent truncation of console file data (see Debbugs above).

[alejandro-colomar]

Changes:

• Don't else after [[noreturn]]

[alejandro-colomar]

Changes:

• Add STRLEN() for fixing VLA issues (strlen(3) doesn't produce a constant expression)

[alejandro-colomar]

Changes:

• Use less bytes for the definition of STRLEN().

[ikerexxe]

Only a minor question.

[alejandro-colomar]

Hi Iker, Paul,

On 3/17/23 11:52, Iker Pedrosa wrote: @ikerexxe commented on this pull request. Only a minor question. > } + + tm = gmtime(&t); + if (tm == NULL) { + (void) strlcpy(buf, "future", size); Why do you assume this is `future`? I'm checking the documentation and I haven't found any reference to it.

Hmm, I'm not sure. This was done by Paul (although I modified the implementation), who knows the time APIs much better than me, so I've CCed him. From what I can see, the only error documented in gmtime(3) is: ERRORS EOVERFLOW The result cannot be represented. Paul, does EOVERFLOW always correspond to far-future times? Cheers, Alex

…

> } + + tm = gmtime(&t); There's no risk here as we only have one thread, so I'm dismissing the alert.

-- <http://www.alejandro-colomar.es/> GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

[alejandro-colomar]

On 3/17/23 09:29, Alejandro Colomar wrote: Paul, does EOVERFLOW always correspond to far-future times?

It does if the time is nonnegative, which as I recall is the case here.

[alejandro-colomar]

On 3/17/23 22:41, Paul Eggert wrote: On 3/17/23 09:29, Alejandro Colomar wrote: > Paul, does EOVERFLOW always correspond to far-future times? It does if the time is nonnegative, which as I recall is the case here.

Thanks! Yes, that's the case.

…

-- <http://www.alejandro-colomar.es/> GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

[ikerexxe]

Thanks for the clarification. Now it makes more sense. LGTM.

[alejandro-colomar]

Changes:

• Rebased to master.
• Add reviewed-by tags

$ git range-diff 6202fb1f^..gh/strlcpy master..strlcpy 
 1:  6202fb1f !  1:  8f4e72ff Simplify change_field() by using strcpy
    @@ Commit message
     
         Signed-off-by: Paul Eggert <eggert@cs.ucla.edu>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
    +    Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
     
      ## lib/fields.c ##
     @@ lib/fields.c: void change_field (char *buf, size_t maxsize, const char *prompt)
 2:  b4a9e1f6 !  2:  537cd8f8 Omit unneeded test in change_field()
    @@ Commit message
     
         Signed-off-by: Paul Eggert <eggert@cs.ucla.edu>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
    +    Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
     
      ## lib/fields.c ##
     @@ lib/fields.c: void change_field (char *buf, size_t maxsize, const char *prompt)
 3:  f24bf2ce !  3:  a867e81a Fix change_field() buffer underrun
    @@ Commit message
     
         Signed-off-by: Paul Eggert <eggert@cs.ucla.edu>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
    +    Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
     
      ## lib/fields.c ##
     @@ lib/fields.c: void change_field (char *buf, size_t maxsize, const char *prompt)
 4:  67be4a8f !  4:  968e60db Prefer strcpy(3) to strlcpy(3) when either works
    @@ Commit message
     
         Signed-off-by: Paul Eggert <eggert@cs.ucla.edu>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
    +    Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
     
      ## lib/gshadow.c ##
     @@ lib/gshadow.c: void endsgent (void)
 5:  10c2d51f !  5:  ef340fe0 Fix crash with large timestamps
    @@ Commit message
         Reported-by: Paul Eggert <eggert@cs.ucla.edu>
         Co-developed-by: Paul Eggert <eggert@cs.ucla.edu>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
    +    Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
     
      ## libmisc/date_to_str.c ##
     @@
 6:  1b95ac98 !  6:  50f9ce97 Add STRLEN(): a constexpr strlen(3) for string literals
    @@ Commit message
         Add STRLEN(): a constexpr strlen(3) for string literals
     
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
    +    Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
     
      ## lib/defines.h ##
     @@ lib/defines.h: static inline void memzero(void *ptr, size_t size)
 7:  5915e8bd !  7:  6c3e9a93 Fix is_my_tty() buffer overrun
    @@ Commit message
         Reported-by: Paul Eggert <eggert@cs.ucla.edu>
         Co-developed-by: Paul Eggert <eggert@cs.ucla.edu>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
    +    Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
     
      ## libmisc/utmp.c ##
     @@
 8:  10596ae1 !  8:  1c77312f Simplify is_my_tty()
    @@ Commit message
     
         Cc: Paul Eggert <eggert@cs.ucla.edu>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
    +    Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
     
      ## libmisc/utmp.c ##
     @@ libmisc/utmp.c: static bool is_my_tty (const char tty[UT_LINESIZE])
 9:  bc55eae6 !  9:  1c0e0124 Fix su(1) silent truncation
    @@ Commit message
         Reported-by: Paul Eggert <eggert@cs.ucla.edu>
         Co-developed-by: Paul Eggert <eggert@cs.ucla.edu>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
    +    Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
     
      ## src/su.c ##
     @@ src/su.c: static /*@only@*/struct passwd * check_perms (void)