mbox series

[v2,0/3] strlcpy and related functions

Message ID cover.1681993374.git.fweimer@redhat.com
Headers
Series strlcpy and related functions |

Message

Florian Weimer April 20, 2023, 12:28 p.m. UTC 
  I split this repost somewhat differently, with the manual updates in a
separate commit, so that I can give proper attribution to Paul's work.

I went through the discussion again and wasn't sure if there was
anything regarding the actual implementation, beyond the fortify macro
usage (discussed with Siddhesh) and Paul's suggestion to open-code the
copying and counting (which I declined).

Thanks,
Florian

Florian Weimer (2):
  Implement strlcpy and strlcat [BZ #178]
  Add the wcslcpy, wcslcat functions

Paul Eggert (1):
  manual: Manual update for strlcat, strlcpy, wcslcat, wclscpy

 NEWS                                          |  3 +
 debug/Makefile                                |  4 +
 debug/Versions                                |  6 ++
 debug/strlcat_chk.c                           | 31 ++++++
 debug/strlcpy_chk.c                           | 31 ++++++
 debug/tst-fortify.c                           | 48 ++++++++++
 debug/wcslcat_chk.c                           | 31 ++++++
 debug/wcslcpy_chk.c                           | 31 ++++++
 include/string.h                              |  4 +
 include/wchar.h                               |  5 +
 manual/maint.texi                             |  8 ++
 manual/string.texi                            | 96 ++++++++++++++++++-
 string/Makefile                               |  4 +
 string/Versions                               |  4 +
 string/bits/string_fortified.h                | 36 +++++++
 string/string.h                               | 13 +++
 string/strlcat.c                              | 59 ++++++++++++
 string/strlcpy.c                              | 46 +++++++++
 string/tst-strlcat.c                          | 84 ++++++++++++++++
 string/tst-strlcpy.c                          | 68 +++++++++++++
 sysdeps/mach/hurd/i386/libc.abilist           |  8 ++
 sysdeps/unix/sysv/linux/aarch64/libc.abilist  |  8 ++
 sysdeps/unix/sysv/linux/alpha/libc.abilist    |  8 ++
 sysdeps/unix/sysv/linux/arc/libc.abilist      |  8 ++
 sysdeps/unix/sysv/linux/arm/be/libc.abilist   |  8 ++
 sysdeps/unix/sysv/linux/arm/le/libc.abilist   |  8 ++
 sysdeps/unix/sysv/linux/csky/libc.abilist     |  8 ++
 sysdeps/unix/sysv/linux/hppa/libc.abilist     |  8 ++
 sysdeps/unix/sysv/linux/i386/libc.abilist     |  8 ++
 sysdeps/unix/sysv/linux/ia64/libc.abilist     |  8 ++
 .../sysv/linux/loongarch/lp64/libc.abilist    |  8 ++
 .../sysv/linux/m68k/coldfire/libc.abilist     |  8 ++
 .../unix/sysv/linux/m68k/m680x0/libc.abilist  |  8 ++
 .../sysv/linux/microblaze/be/libc.abilist     |  8 ++
 .../sysv/linux/microblaze/le/libc.abilist     |  8 ++
 .../sysv/linux/mips/mips32/fpu/libc.abilist   |  8 ++
 .../sysv/linux/mips/mips32/nofpu/libc.abilist |  8 ++
 .../sysv/linux/mips/mips64/n32/libc.abilist   |  8 ++
 .../sysv/linux/mips/mips64/n64/libc.abilist   |  8 ++
 sysdeps/unix/sysv/linux/nios2/libc.abilist    |  8 ++
 sysdeps/unix/sysv/linux/or1k/libc.abilist     |  8 ++
 .../linux/powerpc/powerpc32/fpu/libc.abilist  |  8 ++
 .../powerpc/powerpc32/nofpu/libc.abilist      |  8 ++
 .../linux/powerpc/powerpc64/be/libc.abilist   |  8 ++
 .../linux/powerpc/powerpc64/le/libc.abilist   |  8 ++
 .../unix/sysv/linux/riscv/rv32/libc.abilist   |  8 ++
 .../unix/sysv/linux/riscv/rv64/libc.abilist   |  8 ++
 .../unix/sysv/linux/s390/s390-32/libc.abilist |  8 ++
 .../unix/sysv/linux/s390/s390-64/libc.abilist |  8 ++
 sysdeps/unix/sysv/linux/sh/be/libc.abilist    |  8 ++
 sysdeps/unix/sysv/linux/sh/le/libc.abilist    |  8 ++
 .../sysv/linux/sparc/sparc32/libc.abilist     |  8 ++
 .../sysv/linux/sparc/sparc64/libc.abilist     |  8 ++
 .../unix/sysv/linux/x86_64/64/libc.abilist    |  8 ++
 .../unix/sysv/linux/x86_64/x32/libc.abilist   |  8 ++
 wcsmbs/Makefile                               |  4 +
 wcsmbs/Versions                               |  2 +
 wcsmbs/bits/wchar2.h                          | 37 +++++++
 wcsmbs/tst-wcslcat.c                          | 93 ++++++++++++++++++
 wcsmbs/tst-wcslcpy.c                          | 78 +++++++++++++++
 wcsmbs/wchar.h                                | 13 +++
 wcsmbs/wcslcat.c                              | 60 ++++++++++++
 wcsmbs/wcslcpy.c                              | 46 +++++++++
 63 files changed, 1222 insertions(+), 3 deletions(-)
 create mode 100644 debug/strlcat_chk.c
 create mode 100644 debug/strlcpy_chk.c
 create mode 100644 debug/wcslcat_chk.c
 create mode 100644 debug/wcslcpy_chk.c
 create mode 100644 string/strlcat.c
 create mode 100644 string/strlcpy.c
 create mode 100644 string/tst-strlcat.c
 create mode 100644 string/tst-strlcpy.c
 create mode 100644 wcsmbs/tst-wcslcat.c
 create mode 100644 wcsmbs/tst-wcslcpy.c
 create mode 100644 wcsmbs/wcslcat.c
 create mode 100644 wcsmbs/wcslcpy.c


base-commit: 65cbd52174f5bc211dd655727c2239e25e55bfce

Comments

H.J. Lu April 20, 2023, 4:19 p.m. UTC | #1 
On Thu, Apr 20, 2023 at 5:28 AM Florian Weimer via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> I split this repost somewhat differently, with the manual updates in a
> separate commit, so that I can give proper attribution to Paul's work.
>
> I went through the discussion again and wasn't sure if there was
> anything regarding the actual implementation, beyond the fortify macro
> usage (discussed with Siddhesh) and Paul's suggestion to open-code the
> copying and counting (which I declined).
>
> Thanks,
> Florian
>
> Florian Weimer (2):
>   Implement strlcpy and strlcat [BZ #178]
>   Add the wcslcpy, wcslcat functions
>
> Paul Eggert (1):
>   manual: Manual update for strlcat, strlcpy, wcslcat, wclscpy
>
>  NEWS                                          |  3 +
>  debug/Makefile                                |  4 +
>  debug/Versions                                |  6 ++
>  debug/strlcat_chk.c                           | 31 ++++++
>  debug/strlcpy_chk.c                           | 31 ++++++
>  debug/tst-fortify.c                           | 48 ++++++++++
>  debug/wcslcat_chk.c                           | 31 ++++++
>  debug/wcslcpy_chk.c                           | 31 ++++++
>  include/string.h                              |  4 +
>  include/wchar.h                               |  5 +
>  manual/maint.texi                             |  8 ++
>  manual/string.texi                            | 96 ++++++++++++++++++-
>  string/Makefile                               |  4 +
>  string/Versions                               |  4 +
>  string/bits/string_fortified.h                | 36 +++++++
>  string/string.h                               | 13 +++
>  string/strlcat.c                              | 59 ++++++++++++
>  string/strlcpy.c                              | 46 +++++++++
>  string/tst-strlcat.c                          | 84 ++++++++++++++++
>  string/tst-strlcpy.c                          | 68 +++++++++++++
>  sysdeps/mach/hurd/i386/libc.abilist           |  8 ++
>  sysdeps/unix/sysv/linux/aarch64/libc.abilist  |  8 ++
>  sysdeps/unix/sysv/linux/alpha/libc.abilist    |  8 ++
>  sysdeps/unix/sysv/linux/arc/libc.abilist      |  8 ++
>  sysdeps/unix/sysv/linux/arm/be/libc.abilist   |  8 ++
>  sysdeps/unix/sysv/linux/arm/le/libc.abilist   |  8 ++
>  sysdeps/unix/sysv/linux/csky/libc.abilist     |  8 ++
>  sysdeps/unix/sysv/linux/hppa/libc.abilist     |  8 ++
>  sysdeps/unix/sysv/linux/i386/libc.abilist     |  8 ++
>  sysdeps/unix/sysv/linux/ia64/libc.abilist     |  8 ++
>  .../sysv/linux/loongarch/lp64/libc.abilist    |  8 ++
>  .../sysv/linux/m68k/coldfire/libc.abilist     |  8 ++
>  .../unix/sysv/linux/m68k/m680x0/libc.abilist  |  8 ++
>  .../sysv/linux/microblaze/be/libc.abilist     |  8 ++
>  .../sysv/linux/microblaze/le/libc.abilist     |  8 ++
>  .../sysv/linux/mips/mips32/fpu/libc.abilist   |  8 ++
>  .../sysv/linux/mips/mips32/nofpu/libc.abilist |  8 ++
>  .../sysv/linux/mips/mips64/n32/libc.abilist   |  8 ++
>  .../sysv/linux/mips/mips64/n64/libc.abilist   |  8 ++
>  sysdeps/unix/sysv/linux/nios2/libc.abilist    |  8 ++
>  sysdeps/unix/sysv/linux/or1k/libc.abilist     |  8 ++
>  .../linux/powerpc/powerpc32/fpu/libc.abilist  |  8 ++
>  .../powerpc/powerpc32/nofpu/libc.abilist      |  8 ++
>  .../linux/powerpc/powerpc64/be/libc.abilist   |  8 ++
>  .../linux/powerpc/powerpc64/le/libc.abilist   |  8 ++
>  .../unix/sysv/linux/riscv/rv32/libc.abilist   |  8 ++
>  .../unix/sysv/linux/riscv/rv64/libc.abilist   |  8 ++
>  .../unix/sysv/linux/s390/s390-32/libc.abilist |  8 ++
>  .../unix/sysv/linux/s390/s390-64/libc.abilist |  8 ++
>  sysdeps/unix/sysv/linux/sh/be/libc.abilist    |  8 ++
>  sysdeps/unix/sysv/linux/sh/le/libc.abilist    |  8 ++
>  .../sysv/linux/sparc/sparc32/libc.abilist     |  8 ++
>  .../sysv/linux/sparc/sparc64/libc.abilist     |  8 ++
>  .../unix/sysv/linux/x86_64/64/libc.abilist    |  8 ++
>  .../unix/sysv/linux/x86_64/x32/libc.abilist   |  8 ++
>  wcsmbs/Makefile                               |  4 +
>  wcsmbs/Versions                               |  2 +
>  wcsmbs/bits/wchar2.h                          | 37 +++++++
>  wcsmbs/tst-wcslcat.c                          | 93 ++++++++++++++++++
>  wcsmbs/tst-wcslcpy.c                          | 78 +++++++++++++++
>  wcsmbs/wchar.h                                | 13 +++
>  wcsmbs/wcslcat.c                              | 60 ++++++++++++
>  wcsmbs/wcslcpy.c                              | 46 +++++++++
>  63 files changed, 1222 insertions(+), 3 deletions(-)
>  create mode 100644 debug/strlcat_chk.c
>  create mode 100644 debug/strlcpy_chk.c
>  create mode 100644 debug/wcslcat_chk.c
>  create mode 100644 debug/wcslcpy_chk.c
>  create mode 100644 string/strlcat.c
>  create mode 100644 string/strlcpy.c
>  create mode 100644 string/tst-strlcat.c
>  create mode 100644 string/tst-strlcpy.c
>  create mode 100644 wcsmbs/tst-wcslcat.c
>  create mode 100644 wcsmbs/tst-wcslcpy.c
>  create mode 100644 wcsmbs/wcslcat.c
>  create mode 100644 wcsmbs/wcslcpy.c
>
>
> base-commit: 65cbd52174f5bc211dd655727c2239e25e55bfce
> --
> 2.40.0
>

Should we add benchtests for them?
Florian Weimer April 21, 2023, 5:46 p.m. UTC | #2 
* H. J. Lu:

> Should we add benchtests for them?

Sure, but that's really not my area of expertise.

We could start with the strcpy tests if they exist, and pass down the
buffer sizes.  It would allow us to compare strcpy vs strlcpy, which
might be interesting in its own right.

I don't think it's necessary to benchmark the truncation case.

Thanks,
Florian
Noah Goldstein April 22, 2023, 12:25 a.m. UTC | #3 
On Fri, Apr 21, 2023 at 12:46 PM Florian Weimer via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> * H. J. Lu:
>
> > Should we add benchtests for them?
>
> Sure, but that's really not my area of expertise.
>
> We could start with the strcpy tests if they exist, and pass down the
> buffer sizes.  It would allow us to compare strcpy vs strlcpy, which
> might be interesting in its own right.
>
> I don't think it's necessary to benchmark the truncation case.
>

I posted a patch with benchmarks. Just reuses what we have for
strncpy/strncat. We can use that if you'd like.
> Thanks,
> Florian
>
Florian Weimer April 28, 2023, 10:48 a.m. UTC | #4 
* Florian Weimer via Libc-alpha:

> I split this repost somewhat differently, with the manual updates in a
> separate commit, so that I can give proper attribution to Paul's work.
>
> I went through the discussion again and wasn't sure if there was
> anything regarding the actual implementation, beyond the fortify macro
> usage (discussed with Siddhesh) and Paul's suggestion to open-code the
> copying and counting (which I declined).
>
> Thanks,
> Florian
>
> Florian Weimer (2):
>   Implement strlcpy and strlcat [BZ #178]
>   Add the wcslcpy, wcslcat functions
>
> Paul Eggert (1):
>   manual: Manual update for strlcat, strlcpy, wcslcat, wclscpy

Any further comments on this version?

Thanks,
Florian