Re: [PLUG] this code crashes [OT]

[gabriel rosenkoetter <gr@eclipsed.net>]

On Wed, Jun 19, 2002 at 07:47:09PM -0400, Fred K Ollinger wrote:
> This code will crash:

Well, it's not like I tested it or anything. :^>

> bytes_read = fread(buffer, 1, MAX_BUF, source);
>   /* second arg could be whatever you like; 1 == sizeof(char) in my world
> */
> while(bytes_read) {
>         fwrite(buffer, 1, bytes_read, dest);
> }
> 
> I'm wondering why, I'm guessing that the 4th arg in bytes_read should be
> pointer. I'm going to test that now.

Um, I *think* I'd declared that as a FILE *...

Lessee here, let me dig up the original message from the PLUG
archives.

Ah, I think I see what the bug was, presuming you didn't already fix
it. I was doing source = fopen(source, ...) where we clearly want
source = fopen(src, ...)

Oh, heh, that's not the only problem, since perfectly valid bits lie
after the end of source. Whoops. Mine just went on copying, yours
probably seg fault because it hit unallocated memory, or memory to
which you, the user, didn't have permission.

Okay, try the attached version. I even compiled it and used it.
(Successfully copied the output of dd if=/dev/random ..., so I'm
pretty sure it works.) Short version is, do this instead:

  while(1) {
    bytes_read = fread(buffer, 1, MAX_BUF, source);
    fwrite(buffer, 1, bytes_read, dest);
    if (bytes_read < MAX_BUF) break;
  }

Note that you really need to do a lot of error checking than I do
(dealing with bounds cases on paths especially; people WILL try to
exploit this, because people are assholes like me ;^>) if anyone
else is going to use this software for anything important.

(Remember relative paths! They're hard! I know, I wrote a file
system emulator as part of an OS class. ..s especially so. Also note
my comment on the difference in semblance between // and ///.)

Note that the code for cp(1) is VASTLY more complex, and not just
because it deals with the pathing stuff. There are other cases
(copying to a directory rather than a specific file), permissions,
dealing with special kinds of files like FIFOs and device nodes),
but more than that there are much faster ways to deal with files,
provided you have a halfway decent VM system. If you've got 'em,
check out madvise(2), mlock(2), msync(2), and munmap(2) (may go by
other names, that's what UVM calls them). At the least, have a look
at your mmap(2) and see what else it references.

> I really appreciate all the help, espcially from Gabriel.

Feh. I didn't tell you anything you couldn't have figured out
yourself in a few minutes with the right references, I don't think.

-- 
gabriel rosenkoetter
gr@eclipsed.net

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

#define MAX_BUF 1024

int
main(int argc, char **argv, char **envp)
{
	char *dst, *src;
	char buffer[MAX_BUF];
	size_t bytes_read;
	FILE *source, *dest;

	if (argc != 3) exit(1);

	src = argv[1];
	dst = argv[2];

	/*
	 * Worlds of error checking and /-handling (remember that THREE
	 * initial /s may be collapsed, TWO or fewer MUST be retained)
	 * need to go here.
	 */

	source = fopen(src, "r");
	dest = fopen(dst, "w");
	
	memset(buffer, 0, MAX_BUF);

	while(1) {
		bytes_read = fread(buffer, 1, MAX_BUF, source);
		fwrite(buffer, 1, bytes_read, dest);
		if (bytes_read < MAX_BUF) break;
	}
	
	fclose(dest);
	fclose(source);
}

Attachment: pgps3AOkHMzNg.pgp
Description: PGP signature