coreutils
[Top][All Lists]
Advanced

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

[coreutils] [PATCH 1/2] sort: avoid unnecessary use of sprintf


From: Jim Meyering
Subject: [coreutils] [PATCH 1/2] sort: avoid unnecessary use of sprintf
Date: Tue, 08 Jun 2010 11:42:21 +0200

This first patch removes uses of sprintf in favor of
umaxtostr and stpcpy.

It also adjusts some uses of INT_BUFSIZE_BOUND to use
a variable name rather than a type.  This is for maintainability:
with this change, if the variable type ever changes, the buffer
size does not need to be updated manually (easy to forget).

The second patch does more of the same, in other files.
However, in some of the remaining (unchanged) uses of INT_BUFSIZE_BOUND,
using a variable name is not practical or not an improvement.
There are enough exceptions that I haven't tried to write a
syntax-check for this.

>From f1d5b696858482c58ecc203387c77f048643c19d Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sun, 6 Jun 2010 18:51:42 +0200
Subject: [PATCH 1/2] sort: avoid unnecessary use of sprintf

sprintf is relatively heavy-weight.
* src/sort.c (key_warnings): Use umaxtostr and stpcpy rather
than sprintf.
Also, replace each INT_BUFSIZE_BOUND "type_name" argument
with the equivalent variable name.  More maintainable that way.
---
 src/sort.c |   26 ++++++++++++++------------
 1 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/src/sort.c b/src/sort.c
index 5c3da0c..4c00092 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -1260,7 +1260,7 @@ specify_nmerge (int oi, char c, char const *s)

   if (e == LONGINT_OVERFLOW)
     {
-      char max_nmerge_buf[INT_BUFSIZE_BOUND (unsigned int)];
+      char max_nmerge_buf[INT_BUFSIZE_BOUND (max_nmerge)];
       error (0, 0, _("--%s argument %s too large"),
              long_options[oi].name, quote (s));
       error (SORT_FAILURE, 0,
@@ -2230,27 +2230,29 @@ key_warnings (struct keyfield const *gkey, bool 
gkey_only)
     {
       if (key->obsolete_used)
         {
+          size_t sword = key->sword;
+          size_t eword = key->eword;
+          char tmp[INT_BUFSIZE_BOUND (sword)];
           /* obsolescent syntax +A.x -B.y is equivalent to:
                -k A+1.x+1,B.y   (when y = 0)
                -k A+1.x+1,B+1.y (when y > 0)  */
-          char obuf[INT_BUFSIZE_BOUND (size_t) * 2 + 4]; /* +# -#  */
-          char nbuf[INT_BUFSIZE_BOUND (size_t) * 2 + 5]; /* -k #,#  */
-
+          char obuf[INT_BUFSIZE_BOUND (sword) * 2 + 4]; /* +# -#  */
+          char nbuf[INT_BUFSIZE_BOUND (sword) * 2 + 5]; /* -k #,#  */
           char *po = obuf;
           char *pn = nbuf;

-          size_t sword = key->sword;
-          size_t eword = key->eword;
           if (sword == SIZE_MAX)
             sword++;

-          po += sprintf (po, "+%" PRIuMAX, (uintmax_t) sword);
-          pn += sprintf (pn, "-k %" PRIuMAX, (uintmax_t) sword + 1);
+          po = stpcpy (stpcpy (po, "+"), umaxtostr (sword, tmp));
+          pn = stpcpy (stpcpy (pn, "-k "), umaxtostr (sword + 1, tmp));
           if (key->eword != SIZE_MAX)
             {
-              po += sprintf (po, " -%" PRIuMAX, (uintmax_t) eword + 1);
-              pn += sprintf (pn, ",%" PRIuMAX,
-                             (uintmax_t) eword + 1 + (key->echar == SIZE_MAX));
+              po = stpcpy (stpcpy (po, " -"),
+                           umaxtostr ((uintmax_t) eword + 1, tmp));
+              pn = stpcpy (stpcpy (pn, ","),
+                           umaxtostr ((uintmax_t) eword + 1
+                                      + (key->echar == SIZE_MAX), tmp));
             }
           error (0, 0, _("obsolescent key `%s' used; consider `%s' instead"),
                  obuf, nbuf);
@@ -2651,7 +2653,7 @@ check (char const *file_name, char checkonly)
                 struct line const *disorder_line = line - 1;
                 uintmax_t disorder_line_number =
                   buffer_linelim (&buf) - disorder_line + line_number;
-                char hr_buf[INT_BUFSIZE_BOUND (uintmax_t)];
+                char hr_buf[INT_BUFSIZE_BOUND (disorder_line_number)];
                 fprintf (stderr, _("%s: %s:%s: disorder: "),
                          program_name, file_name,
                          umaxtostr (disorder_line_number, hr_buf));
--
1.7.1.501.g23b46


>From 1108856103acec0ed448757ef3ee2f88857432fc Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Mon, 7 Jun 2010 07:15:15 +0200
Subject: [PATCH 2/2] maint: adjust INT_BUFSIZE_BOUND usage for maintainability

* src/tail.c (xlseek): Give INT_BUFSIZE_BOUND a variable name,
not a type name.
* src/ls.c (gobble_file, format_user_or_group_width): Likewise.
* src/head.c (elide_tail_bytes_pipe): Likewise.
(elide_tail_lines_seekable, main): Likewise.
---
 src/head.c |    8 ++++----
 src/ls.c   |    4 ++--
 src/tail.c |    2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/head.c b/src/head.c
index 7fa5823..dd3dc89 100644
--- a/src/head.c
+++ b/src/head.c
@@ -226,7 +226,7 @@ elide_tail_bytes_pipe (const char *filename, int fd, 
uintmax_t n_elide_0)

   if (SIZE_MAX < n_elide_0 + READ_BUFSIZE)
     {
-      char umax_buf[INT_BUFSIZE_BOUND (uintmax_t)];
+      char umax_buf[INT_BUFSIZE_BOUND (n_elide_0)];
       error (EXIT_FAILURE, 0, _("%s: number of bytes is too large"),
              umaxtostr (n_elide_0, umax_buf));
     }
@@ -611,7 +611,7 @@ elide_tail_lines_seekable (const char *pretty_filename, int 
fd,
   pos -= bytes_read;
   if (lseek (fd, pos, SEEK_SET) < 0)
     {
-      char offset_buf[INT_BUFSIZE_BOUND (off_t)];
+      char offset_buf[INT_BUFSIZE_BOUND (pos)];
       error (0, errno, _("%s: cannot seek to offset %s"),
              pretty_filename, offtostr (pos, offset_buf));
       return false;
@@ -682,7 +682,7 @@ elide_tail_lines_seekable (const char *pretty_filename, int 
fd,
       pos -= BUFSIZ;
       if (lseek (fd, pos, SEEK_SET) < 0)
         {
-          char offset_buf[INT_BUFSIZE_BOUND (off_t)];
+          char offset_buf[INT_BUFSIZE_BOUND (pos)];
           error (0, errno, _("%s: cannot seek to offset %s"),
                  pretty_filename, offtostr (pos, offset_buf));
           return false;
@@ -1042,7 +1042,7 @@ main (int argc, char **argv)

   if ( ! count_lines && elide_from_end && OFF_T_MAX < n_units)
     {
-      char umax_buf[INT_BUFSIZE_BOUND (uintmax_t)];
+      char umax_buf[INT_BUFSIZE_BOUND (n_units)];
       error (EXIT_FAILURE, 0, _("%s: number of bytes is too large"),
              umaxtostr (n_units, umax_buf));
     }
diff --git a/src/ls.c b/src/ls.c
index 9549130..4e0a036 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -2986,7 +2986,7 @@ gobble_file (char const *name, enum filetype type, ino_t 
inode,

       if (format == long_format)
         {
-          char b[INT_BUFSIZE_BOUND (uintmax_t)];
+          char b[INT_BUFSIZE_BOUND (f->stat.st_nlink)];
           int b_len = strlen (umaxtostr (f->stat.st_nlink, b));
           if (nlink_width < b_len)
             nlink_width = b_len;
@@ -3582,7 +3582,7 @@ format_user_or_group_width (char const *name, unsigned 
long int id)
     }
   else
     {
-      char buf[INT_BUFSIZE_BOUND (unsigned long int)];
+      char buf[INT_BUFSIZE_BOUND (id)];
       sprintf (buf, "%lu", id);
       return strlen (buf);
     }
diff --git a/src/tail.c b/src/tail.c
index 9e95dee..75e9d53 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -434,7 +434,7 @@ static off_t
 xlseek (int fd, off_t offset, int whence, char const *filename)
 {
   off_t new_offset = lseek (fd, offset, whence);
-  char buf[INT_BUFSIZE_BOUND (off_t)];
+  char buf[INT_BUFSIZE_BOUND (offset)];
   char *s;

   if (0 <= new_offset)
--
1.7.1.501.g23b46



reply via email to

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