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).