cp /proc/cpuinfo

Version:

coreutils-6.8 (fixed in 6.9)

How to reproduce?

coreutils-6.8/src/cp /proc/cpuinfo ./cpuinfo

Symptom:

Incorrect results.

Instead of copying the cpuinfo into the file “./cpuinfo”, “cp” simply created an empty file “./cpuinfo”.

The correct behavior should be for “cp” to copy this file with its content.

Root cause:

The buggy version contains a premature optimization, that was to omit the actual content copying if the file is regular file and size is zero. It didn’t expect the ‘cpuinfo’ being such a file whose content is generated on demand, thus omitted the actual content write. The fix is simply remove this premature optimization.

The bug occurred in function “copy_reg”, which is the worker function that actually performs the copy job.

----------------------- copy.c (comments written by us in blue color) -------------------------

copy_reg (src_name=0x7fff4bb0a9c9 "/proc/cpuinfo", dst_name=0x7fff4bb0a9d7 "./c", x=0x7fff4bb08770, dst_mode=292, omitted_permissions=0, new_dst=0x7fff4bb0816c, src_sb=0x7fff4bb08340) {

   … …

   /* The following logic is a premature optimization: if the size of

    * the source file is zero and it is a regular file, cp will not

    * actually copy the file content. However, for the special file:

    * /proc/cpuinfo, its file size returned by ‘stat’ system call is zero,

    * and it is a regular file, but its content will be generated

    * dynamically when ‘read’ it. So during the buggy execution, ‘cp’ s

    * imply ignored this file and skipped the actual

    * content copying. The fix is simple: remove the premature optimization.

    */

-  if (! (S_ISREG (src_open_sb.st_mode) && src_open_sb.st_size == 0))

   {

     ... ...

     /* Make a buffer with space for a sentinel at the end.  */

     buf_alloc = xmalloc (buf_size + buf_alignment_slop);

     buf = ptr_align (buf_alloc, buf_alignment);

     for (;;) {

          ssize_t n_read = read (source_desc, buf, buf_size);

             … ...

              if (full_write (dest_desc, buf, n) != n)

                {

                  error (0, errno, _("writing %s"), quote (dst_name));

                  return_val = false;

                  goto close_src_and_dst_desc;

                }

              last_write_made_hole = false;

              /* A short read on a regular file means EOF.  */

              if (n_read != buf_size && S_ISREG (src_open_sb.st_mode))

                break;

        

        }// for

     ....

-    } //if (! (S_ISREG (src_open_sb.st_mode)

} // copy_reg

--------------------------------------------------------------------------------------------------------------

Is there Error Message?

No.

Errlog’s log insertion explained

Errlog can insert a message since this false branch of this ‘if’ decision:

 if (! (S_ISREG (src_open_sb.st_mode) && src_open_sb.st_size == 0)) {

          /* perform the full content copy. */

    }

+   else

+      elog (“untested branch decision”);

is not tested.

We also annotate the “stat” system call’s return code that if the file size is 0, we print a warning message (as we empirically found many such cases are suspicious). So Errlog can also print an error message via “system call error return” pattern.

Discussion:

This is yet another case of: “premature optimization is the root of all devil”. So the developers could have printed a warning message when the optimization condition is met:

 

   if (! (S_ISREG (src_open_sb.st_mode) && src_open_sb.st_size == 0)) {

          /* perform the full content copy. */

    }

+   else

+      /* The optimization condition is met, and we are skipping the actual content copy: */

+      log (“warning: file %s is a regular file with size 0, and we skipped the copy”, filename);

If the developers can print such an error message, then it is trivial to diagnose the failure! In fact, we have seen such practices of putting warning messages in not-well tested code and remove them after the code hardens in PostgreSQL (http://archives.postgresql.org/pgsql-patches/2006-09/msg00293.php).