crusade against strncpy/strcpy ;-)

[Jim Meyering]

strncpy is particularly bad, but even strcpy is best avoided.
There are still a few uses remaining, and these remove a few from ls.c
and tac.c.  As an added bonus, one part of ls.c no longer hard-codes "/"
as file name component separator (though note that my motivation for that
change was to avoid rolling my own, rather than to support an alternate
separator).

>From 280aa28c4dc82f394d423bfceaac29b5b48c3cd2 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 18 Apr 2012 13:36:11 +0200
Subject: [PATCH 1/3] maint: modernize/clean-up a small function in ls.c

* src/ls.c (make_link_name): Adjust comment style to refer to VARIABLE
names, not 'variable'.
Move each of two declarations "down" to first use.
Compare pointer to NULL, not to 0.
Don't reuse local, "linkbuf" for a different purpose.
---
 src/ls.c |   25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/src/ls.c b/src/ls.c
index f1dfb1e..db58192 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -3193,17 +3193,14 @@ get_link_name (char const *filename, struct fileinfo 
*f, bool command_line_arg)
                   filename);
 }

-/* If 'linkname' is a relative name and 'name' contains one or more
-   leading directories, return 'linkname' with those directories
-   prepended; otherwise, return a copy of 'linkname'.
-   If 'linkname' is zero, return zero.  */
+/* If LINKNAME is a relative name and NAME contains one or more
+   leading directories, return LINKNAME with those directories
+   prepended; otherwise, return a copy of LINKNAME.
+   If LINKNAME is NULL, return NULL.  */

 static char *
 make_link_name (char const *name, char const *linkname)
 {
-  char *linkbuf;
-  size_t bufsiz;
-
   if (!linkname)
     return NULL;

@@ -3212,15 +3209,15 @@ make_link_name (char const *name, char const *linkname)

   /* The link is to a relative name.  Prepend any leading directory
      in 'name' to the link name.  */
-  linkbuf = strrchr (name, '/');
-  if (linkbuf == 0)
+  char const *linkbuf = strrchr (name, '/');
+  if (linkbuf == NULL)
     return xstrdup (linkname);

-  bufsiz = linkbuf - name + 1;
-  linkbuf = xmalloc (bufsiz + strlen (linkname) + 1);
-  strncpy (linkbuf, name, bufsiz);
-  strcpy (linkbuf + bufsiz, linkname);
-  return linkbuf;
+  size_t bufsiz = linkbuf - name + 1;
+  char *p = xmalloc (bufsiz + strlen (linkname) + 1);
+  strncpy (p, name, bufsiz);
+  strcpy (p + bufsiz, linkname);
+  return p;
 }

 /* Return true if the last component of NAME is '.' or '..'
--
1.7.10.208.gb4267


>From bcb9078e380d81be62e8df00c45afc2f57392dc2 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 18 Apr 2012 14:49:22 +0200
Subject: [PATCH 2/3] maint: ls: use stpncpy/stpcpy, not strncpy/strcpy

* src/ls.c (gobble_file): Move a decl "down".
(make_link_name): Do not hard-code '/'.  Use IS_ABSOLUTE_FILE_NAME
and dir_len instead.
Use stpcpy/stpncpy in place of strncpy/strcpy.
---
 src/ls.c |   15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/src/ls.c b/src/ls.c
index db58192..800f813 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -3037,11 +3037,10 @@ gobble_file (char const *name, enum filetype type, 
ino_t inode,
       if (S_ISLNK (f->stat.st_mode)
           && (format == long_format || check_symlink_color))
         {
-          char *linkname;
           struct stat linkstats;

           get_link_name (absolute_name, f, command_line_arg);
-          linkname = make_link_name (absolute_name, f->linkname);
+          char *linkname = make_link_name (absolute_name, f->linkname);

           /* Avoid following symbolic links when possible, ie, when
              they won't be traced and when no indicator is needed.  */
@@ -3204,19 +3203,17 @@ make_link_name (char const *name, char const *linkname)
   if (!linkname)
     return NULL;

-  if (*linkname == '/')
+  if (IS_ABSOLUTE_FILE_NAME (linkname))
     return xstrdup (linkname);

   /* The link is to a relative name.  Prepend any leading directory
      in 'name' to the link name.  */
-  char const *linkbuf = strrchr (name, '/');
-  if (linkbuf == NULL)
+  size_t prefix_len = dir_len (name);
+  if (prefix_len == 0)
     return xstrdup (linkname);

-  size_t bufsiz = linkbuf - name + 1;
-  char *p = xmalloc (bufsiz + strlen (linkname) + 1);
-  strncpy (p, name, bufsiz);
-  strcpy (p + bufsiz, linkname);
+  char *p = xmalloc (prefix_len + 1 + strlen (linkname) + 1);
+  stpcpy (stpncpy (p, name, prefix_len + 1), linkname);
   return p;
 }

--
1.7.10.208.gb4267


>From 219b894de4c366496ffdf3591389de074bfdb2b9 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Thu, 19 Apr 2012 11:32:04 +0200
Subject: [PATCH 3/3] maint: tac: use memcpy, not strcpy

* src/tac.c (main): Use memcpy, not strcpy, since we know the length.
---
 src/tac.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/tac.c b/src/tac.c
index b50382d..2bf8ad2 100644
--- a/src/tac.c
+++ b/src/tac.c
@@ -664,7 +664,7 @@ main (int argc, char **argv)
   G_buffer = xmalloc (G_buffer_size);
   if (sentinel_length)
     {
-      strcpy (G_buffer, separator);
+      memcpy (G_buffer, separator, sentinel_length + 1);
       G_buffer += sentinel_length;
     }
   else
--
1.7.10.208.gb4267