Re: An error in the strcat (3) man page

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Eric,

On 4/6/19 6:34 PM, Eric Sanchis wrote:
Dear Maintener,

I would suggest removing the EXAMPLE part for the following reasons:

1) There is no evidence that in real projects the use of functions
like stpcpy has a great influence on the overall performance. To my
knowledge there is no statistical study made on actual C projetcs
which demonstates that point.

I don't know of any such studies, but I'm sure some people do
naively misuse strcat/strncat without realizing the
performance implications, especially if they come from a
language with better string types. So, I think the example is
worthwhile.

2) Optimization should come after safety. For example, the mystrcat()
function proposed by Joel Spolsky:

char* mystrcat( char* dest, char* src )
{
      while (*dest) dest++;
      while (*dest++ = *src++);
      return --dest;
}
has the same safety level than strcat (NULL pointer, space problem,
etc.). It cannot detect string truncation and does not prevent from
buffer overflow.

3) Examples used to promote functions like stpcpy or mystrcat are
often meaningless. For example,

   a) the example used by Joel:

     char bigString[1000];     /* I never know how much to allocate... */
     bigString[0] = '\0';
     strcat(bigString,"John, ");
     strcat(bigString,"Paul, ");
     strcat(bigString,"George, ");
     strcat(bigString,"Joel ");

concatenates only a few little strings. Running in O(n2) is not a big problem.

   b) the example illustrating the strcat function man page is artificial:

int
main(int argc, char *argv[])
{
#define LIM 4000000
     int j;
     char p[LIM];

     p[0] = '\0' ;

     for (j = 0; j < LIM; j++) {
         strcat(p, "a");
     }
}

The use of memset would be more appropriate:
     memset(p,'a',LIM-1) ;
     p[LIM-1] = '\0' ;

4) Last, but not least: it produces a buffer owerflow (off-by-one
error). A typical trap of strcat.

The program below shows the problem:

#include <string.h>
#include <stdio.h>

        int
        main(int argc, char *argv[])
        {
        #define LIM 4
            char p[LIM];
        int j ;

            p[0] = '\0';

            for (j = 0; j < LIM; j++)
                strcat(p, "a");
        printf("(j=%d)  p='%s' -- psize : %d  -- plen=%u\n", j,p,LIM,
(unsigned int) strlen(p)) ;

            return 0 ;
        }

Execution:

$ prog
(j=4)  p='aaaa' -- psize : 4 -- plen=4
$

p should be 'aaa'.

I asked myself who added that code, and took a look at the Git
history

<blush> :-}

I fixed that as follows:

[[
diff --git a/man3/strcat.3 b/man3/strcat.3
index 7d0998fc1..9263c7ec9 100644
--- a/man3/strcat.3
+++ b/man3/strcat.3
@@ -209,7 +209,7 @@ main(int argc, char *argv[])
 {
 #define LIM 4000000
     int j;
-    char p[LIM];
+    char p[LIM + 1];    /* +1 for terminating null byte */
     time_t base;

     base = time(NULL);
]]

To conclude, I would suggest using the strntcat() or strtcat()
functions for concatenation [cf. tarball]. The first function doesn't
allow truncation (nt: no trunc); the second allows truncation.
They never
    - produce bad-formed strings or buffer overflow.
    - unexpectedly truncate strings.
They respect sound design principles:
    - consistency: in similar situation, behaviors are similar.
    - neutrality: after an error, input data are not modified.
    - robustness: they perform correctly under normal and unexpected situations.
They are easy to use.

I hope you will test them.

Yes, but these manual pages are about documenting the standard C
library...

Thank you for the bug report.

Cheers,

Michael



[Index of Archives]     [Kernel Documentation]     [Netdev]     [Linux Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux