diff -ruw bincimap-1.0.24-dist/src/bincimapd-copy.cc bincimap-1.0.24/src/bincimapd-copy.cc --- bincimap-1.0.24-dist/src/bincimapd-copy.cc 2003-03-01 02:46:46.000000000 -0800 +++ bincimap-1.0.24/src/bincimapd-copy.cc 2003-03-01 03:10:04.000000000 -0800 @@ -123,103 +123,100 @@ } } - // this code copies a file with read and write - FILE *fpread = depot->fopen(mr->getID()); - if (fpread == NULL) { - logger << "COPY couldn't open " << mr->getID() << endl; - continue; - } - + /* Create a tmp/ write filename */ string safenamestr = maildir + "/tmp/bincimap-copy-XXXXXX"; - char *safename = new char[safenamestr.length() + 1]; - strcpy(safename, safenamestr.c_str()); - int fd = mkstemp(safename); - int c; + /* convert it to a c string */ + size_t safenamelength = safenamestr.length() + 1; + /* TODO: Check this against _PC_PATH_MAX */ + /* Here we take advantage of C++'s stack allocation of arrays --cld */ + char safename[safenamelength]; + memset(safename, 0, safenamelength); + strncpy(safename, safenamestr.c_str(), safenamelength-1); - if (fseek(fpread, 0, SEEK_END) == -1) { - logger << "COPY couldn't determine size of " << mr->getID() - << ": " << strerror(errno) << endl; - fclose(fpread); - unlink(safename); - delete safename; - continue; - } - - int copylength = ftell(fpread); - if (copylength == -1) { - fclose(fpread); - unlink(safename); - delete safename; - logger << "COPY couldn't determine size of " << mr->getID() - << ": " << strerror(errno) << endl; - continue; + int fd = mkstemp(safename); + /* see if mkstemp() failed highly unlikely but not impossible --cld */ + if (fd == -1) { + logger << "COPY unable to create temp file '" << safenamestr + << "', unrecoverable error:" << strerror(errno) << endl; + continue; /* continue with next copy (I think) --cld */ } - if (fseek(fpread, 0, SEEK_SET) == -1) { - fclose(fpread); + int fdread = depot->open(mr->getID()); + if(fdread == -1) { + logger << "COPY couldn't open " << mr->getID() << endl; unlink(safename); - delete safename; - logger << "COPY couldn't determine size of " << mr->getID() - << ": " << strerror(errno) << endl; continue; } - char *copybuffer = new char[8192]; + /* Query the OS to find out the best size for I/O on that file --cld */ + size_t buffsz = (size_t)fpathconf(fdread, _PC_PIPE_BUF); - int wrotesize = 0; - bool broke = false; - while (wrotesize < copylength) { - int nbytes = (copylength - wrotesize) < 8192 ? - copylength - wrotesize : 8192; - - if (fread(copybuffer, nbytes, 1, fpread) != 1) { - fclose(fpread); + /* double our buffer size for added sexyness --cld */ + /* TODO: This may not be a good idea, writes should stall at + * _PC_PIPE_BUF anyway so having a larger buffer doesn't gain + * anything. I don't have any data either way. I'm leaving + * it for now. + */ + buffsz = 2*buffsz < SSIZE_MAX ? 2*buffsz : buffsz; + + /* again, a stack buffer to ensure automatic cleanup --cld */ + char copybuffer[buffsz]; + ssize_t bytesread = 0; /* the number of bytes read each cycle */ + bool broke = false; /* ABEND flag from while loop --cld */ + + /* At this point, fdread needs to be copied safely to fd */ + while (!broke && (bytesread = read(fdread, copybuffer, buffsz)) != 0) { + + /* if a failure occurs in this loop, remove the file, log + a message, set broke = true and break; */ + + if (bytesread == -1) { + /* an error occured in the read. --cld */ + close(fdread); unlink(safename); - delete safename; - delete copybuffer; - logger << "COPY couldn't read " << mr->getID() + logger << "COPY failed during read() of " << mr->getID() << ": " << strerror(errno) << endl; broke = true; break; } - int wrote; - int offset = 0; - int left = nbytes; - while ((wrote = write(fd, copybuffer + offset, left)) != left) { - if (errno != EINTR) { - fclose(fpread); + ssize_t byteswritten = 0; + /* attempt to write the entire contents of the buffer */ + while (byteswritten != bytesread) { + ssize_t thesebytes = + write(fd, copybuffer + byteswritten, bytesread-byteswritten); + if(thesebytes == -1) { + /* even if errno is EINTR we have no way of knowing the number + * of bytes actually written so we must abort, on systems with + * safe interruption of reads, we wouldn't get EINTR anyway. --cld + */ + close(fdread); unlink(safename); - delete safename; - delete copybuffer; logger << "COPY couldn't read " << mr->getID() << ": " << strerror(errno) << endl; broke = true; break; } else { - offset += wrote; - left -= wrote; + byteswritten += thesebytes; } } - - wrotesize += nbytes; } if (broke) continue; - delete copybuffer; - - fclose(fpread); - if (fsync(fd) != 0) { + if(errno != EINVAL) { /* EINVAL simply means the FS doesn't do sync */ // recovery + logger << "COPY fsync failed " << strerror(errno) << endl; int err = errno; close(fd); unlink(safename); - delete safename; com << "* NO COPY failed: " << strerror(err) << endl; continue; } + } + + /* at this point, the tmp/ file has been created, now we deliver --cld */ char hostname[512]; int hostnamelen = gethostname(hostname, sizeof(hostname)); @@ -232,13 +229,14 @@ if (close(fd) != 0) { int err = errno; unlink(safename); - delete safename; com << "* NO couldn't close or link file: " << strerror(err) << endl; - logger << "COPY couldn't close or link file: " << safename << ": " << strerror(err) << endl; + logger << "COPY couldn't close or link file: " << (char*)safename + << ": " << strerror(err) << endl; return; } bool copied = false; + /* loop trying to deliver into new/ */ for (int i = 0; i < 1000; ++i) { #ifdef HAVE_SSTREAM stringstream ss; @@ -270,17 +268,15 @@ if (!copied) { int err = errno; unlink(safename); - delete safename; com << "* NO couldn't close or link file: " << strerror(err) << endl; - logger << "COPY couldn't close or link file: " << safename << ": " << strerror(err) << endl; + logger << "COPY couldn't close or link file: " << (char*)safename + << ": " << strerror(err) << endl; return; } if (unlink(safename) != 0) - logger << "COPY warning: couldn't unlink " << safename << ": " - << strerror(errno) << endl; - - delete safename; + logger << "COPY warning: couldn't unlink " << (char*)safename + << ": " << strerror(errno) << endl; } while ((mr = depot->getNextMessage()) != depot->endMessage()); Only in bincimap-1.0.24/src: tags