[Patch] Add option to write to tempfiles and rename when complete

[guyharris]

Converted from SourceForge issue 1892841, submitted by fatkiss

This patch adds the option "-b" to tcpdump. When used together with the -w and -C options, tcpdump will write to file.tmp, and move it to file when done. Then it will write to file1.tmp, and move it to file1 when done, etc.

The resulting files have the same name as before, but you know which files tcpdump is and isn't writing to. Very useful for scripts trying to do something with the output.

Without the -b option, tcpdump's behaviour is unchanged.

-b was selected because none of the other available options made any more sense.

The patch includes an update to the man page.

This patch also checks the result of the malloc() at the beginning of MakeFilename(), it was assumed to work before.

[infrastation]

The suggested change:

Index: tcpdump.1
===================================================================
RCS file: /tcpdump/master/tcpdump/tcpdump.1,v
retrieving revision 1.188
diff -u -r1.188 tcpdump.1
--- tcpdump.1   7 Jan 2008 00:16:06 -0000   1.188
+++ tcpdump.1   13 Feb 2008 15:22:09 -0000
@@ -29,7 +29,7 @@
 .na
 .B tcpdump
 [
-.B \-AdDefKlLnNOpqRStuUvxX
+.B \-AbdDefKlLnNOpqRStuUvxX
 ] [
 .B \-c
 .I count
@@ -259,6 +259,14 @@
 Print each packet (minus its link level header) in ASCII.  Handy for
 capturing web pages.
 .TP
+.B \-b
+When writing to a file with -w, always write to a temporary file first,
+and on successful completion (ie. file size met with -C option), rename
+tmp file to file name, and continue. This should ensure that a cap-file
+is always complete. This is good for knowing when to further process a
+file. The temporary files are named \fIfile.tmp\fP, and are renamed to
+\fIfile\fP, where \fIfile\fP is the name specified with the -w option.
+.TP
 .B \-c
 Exit after receiving \fIcount\fP packets.
 .TP
Index: tcpdump.c
===================================================================
RCS file: /tcpdump/master/tcpdump/tcpdump.c,v
retrieving revision 1.276
diff -u -r1.276 tcpdump.c
--- tcpdump.c   29 Jan 2008 10:49:27 -0000  1.276
+++ tcpdump.c   13 Feb 2008 15:22:09 -0000
@@ -93,6 +93,7 @@
 int dflag;         /* print filter code */
 int Lflag;         /* list available data link types and exit */
 char *zflag = NULL;        /* compress each savefile using a specified command (like gzip or bzip2) */
+int bflag;         /* write to tempfiles and rename on completion */

 static int infodelay;
 static int infoprint;
@@ -305,6 +306,7 @@
 struct dump_info {
    char    *WFileName;
    char    *CurrentFileName;
+   char    *TempFileName;
    pcap_t  *pd;
    pcap_dumper_t *p;
 };
@@ -430,9 +432,11 @@


 static void
-MakeFilename(char *buffer, char *orig_name, int cnt, int max_chars)
+MakeFilename(char *buffer, char *orig_name, int cnt, int max_chars, int tmpname)
 {
         char *filename = malloc(NAME_MAX + 1);
+   if (filename == NULL)
+       error("MakeFilename: malloc");

         /* Process with strftime if Gflag is set. */
         if (Gflag != 0) {
@@ -451,12 +455,23 @@
           strncpy(filename, orig_name, NAME_MAX);
         }

-   if (cnt == 0 && max_chars == 0)
-       strncpy(buffer, filename, NAME_MAX + 1);
-   else
-       if (snprintf(buffer, NAME_MAX + 1, "%s%0*d", filename, max_chars, cnt) > NAME_MAX)
-                  /* Report an error if the filename is too large */
-                  error("too many output files or filename is too long (> %d)", NAME_MAX);
+   if (cnt == 0 && max_chars == 0) {
+       if (tmpname == 0) {
+           strncpy(buffer, filename, NAME_MAX + 1);
+       } else {
+           snprintf(buffer, NAME_MAX + 1, "%s.tmp", filename);
+       }
+   } else {
+       if (tmpname == 0) {
+           if (snprintf(buffer, NAME_MAX + 1, "%s%0*d", filename, max_chars, cnt) > NAME_MAX)
+                     /* Report an error if the filename is too large */
+                     error("too many output files or filename is too long (> %d)", NAME_MAX);
+       } else {
+           if (snprintf(buffer, NAME_MAX + 1, "%s%0*d.tmp", filename, max_chars, cnt) > NAME_MAX)
+           /* Report an error if the filename is too large */
+                     error("too many output files or filename is too long (> %d)", NAME_MAX);
+       }
+   }
         free(filename);
 }

@@ -516,6 +531,7 @@
    infile = NULL;
    RFileName = NULL;
    WFileName = NULL;
+   bflag = 0;
    if ((cp = strrchr(argv[0], '/')) != NULL)
        program_name = cp + 1;
    else
@@ -530,7 +546,7 @@

    opterr = 0;
    while (
-       (op = getopt(argc, argv, "aA" B_FLAG "c:C:d" D_FLAG "eE:fF:G:i:KlLm:M:nNOpqr:Rs:StT:u" U_FLAG "vw:W:xXy:Yz:Z:")) != -1)
+       (op = getopt(argc, argv, "aAb" B_FLAG "c:C:d" D_FLAG "eE:fF:G:i:KlLm:M:nNOpqr:Rs:StT:u" U_FLAG "vw:W:xXy:Yz:Z:")) != -1)
        switch (op) {

        case 'a':
@@ -541,6 +557,10 @@
            ++Aflag;
            break;

+       case 'b':
+           ++bflag;
+           break;
+
 #ifdef WIN32
        case 'B':
            UserBufferSize = atoi(optarg)*1024;
@@ -1026,17 +1046,26 @@
        pcap_dumper_t *p;
        /* Do not exceed the default NAME_MAX for files. */
        dumpinfo.CurrentFileName = (char *)malloc(NAME_MAX + 1);
-
        if (dumpinfo.CurrentFileName == NULL)
            error("malloc of dumpinfo.CurrentFileName");

+       dumpinfo.TempFileName = (char *)malloc(NAME_MAX + 1);
+       if (dumpinfo.TempFileName == NULL)
+           error("malloc of dumpinfo.TempFileName");
+
        /* We do not need numbering for dumpfiles if Cflag isn't set. */
-       if (Cflag != 0)
-         MakeFilename(dumpinfo.CurrentFileName, WFileName, 0, WflagChars);
-       else
-         MakeFilename(dumpinfo.CurrentFileName, WFileName, 0, 0);
+       if (Cflag != 0) {
+         MakeFilename(dumpinfo.CurrentFileName, WFileName, 0, WflagChars, 0);
+         MakeFilename(dumpinfo.TempFileName, WFileName, 0, WflagChars, bflag);
+       } else {
+         MakeFilename(dumpinfo.CurrentFileName, WFileName, 0, 0, 0);
+         MakeFilename(dumpinfo.TempFileName, WFileName, 0, 0, bflag);
+       }

-       p = pcap_dump_open(pd, dumpinfo.CurrentFileName);
+       if (bflag != 0)
+           p = pcap_dump_open(pd, dumpinfo.TempFileName);
+       else
+           p = pcap_dump_open(pd, dumpinfo.CurrentFileName);
        if (p == NULL)
            error("%s", pcap_geterr(pd));
        if (Cflag != 0 || Gflag != 0) {
@@ -1314,6 +1343,11 @@
            if (zflag != NULL)
                compress_savefile(dump_info->CurrentFileName);

+           if (bflag != 0) {
+               if (rename(dump_info->TempFileName, dump_info->CurrentFileName) != 0)
+                   error("dump_packet_and_trunc: rename");
+           }
+
            /*
             * Check to see if we've exceeded the Wflag (when
             * not using Cflag).
@@ -1330,18 +1364,33 @@
            dump_info->CurrentFileName = (char *)malloc(NAME_MAX + 1);
            if (dump_info->CurrentFileName == NULL)
                error("dump_packet_and_trunc: malloc");
+
+           if (dump_info->TempFileName != NULL)
+               free(dump_info->TempFileName);
+           /* Allocate space for max filename + \0. */
+           dump_info->TempFileName = (char *)malloc(NAME_MAX + 1);
+           if (dump_info->TempFileName == NULL)
+               error("dump_packet_and_trunc: malloc");
+
            /*
             * This is always the first file in the Cflag
             * rotation: e.g. 0
             * We also don't need numbering if Cflag is not set.
             */
-           if (Cflag != 0)
+           if (Cflag != 0) {
                MakeFilename(dump_info->CurrentFileName, dump_info->WFileName, 0,
-                   WflagChars);
-           else
-               MakeFilename(dump_info->CurrentFileName, dump_info->WFileName, 0, 0);
+                   WflagChars, 0);
+               MakeFilename(dump_info->TempFileName, dump_info->WFileName, 0,
+                   WflagChars, bflag);
+           } else {
+               MakeFilename(dump_info->CurrentFileName, dump_info->WFileName, 0, 0, 0);
+               MakeFilename(dump_info->TempFileName, dump_info->WFileName, 0, 0, bflag);
+           }

-           dump_info->p = pcap_dump_open(dump_info->pd, dump_info->CurrentFileName);
+           if (bflag != 0)
+               dump_info->p = pcap_dump_open(dump_info->pd, dump_info->TempFileName);
+           else
+               dump_info->p = pcap_dump_open(dump_info->pd, dump_info->CurrentFileName);
            if (dump_info->p == NULL)
                error("%s", pcap_geterr(pd));
        }
@@ -1364,18 +1413,35 @@
        if (zflag != NULL)
            compress_savefile(dump_info->CurrentFileName);

+       if (bflag != 0) {
+           if (rename(dump_info->TempFileName, dump_info->CurrentFileName) != 0)
+               error("dump_packet_and_trunc: rename");
+       }
+
        Cflag_count++;
        if (Wflag > 0) {
            if (Cflag_count >= Wflag)
                Cflag_count = 0;
        }
+
        if (dump_info->CurrentFileName != NULL)
            free(dump_info->CurrentFileName);
        dump_info->CurrentFileName = (char *)malloc(NAME_MAX + 1);
        if (dump_info->CurrentFileName == NULL)
            error("dump_packet_and_trunc: malloc");
-       MakeFilename(dump_info->CurrentFileName, dump_info->WFileName, Cflag_count, WflagChars);
-       dump_info->p = pcap_dump_open(dump_info->pd, dump_info->CurrentFileName);
+       MakeFilename(dump_info->CurrentFileName, dump_info->WFileName, Cflag_count, WflagChars, 0);
+
+       if (dump_info->TempFileName != NULL)
+           free(dump_info->TempFileName);
+       dump_info->TempFileName = (char *)malloc(NAME_MAX + 1);
+       if (dump_info->TempFileName == NULL)
+           error("dump_packet_and_trunc: malloc");
+       MakeFilename(dump_info->TempFileName, dump_info->WFileName, Cflag_count, WflagChars, bflag);
+
+       if (bflag != 0)
+           dump_info->p = pcap_dump_open(dump_info->pd, dump_info->TempFileName);
+       else
+           dump_info->p = pcap_dump_open(dump_info->pd, dump_info->CurrentFileName);
        if (dump_info->p == NULL)
            error("%s", pcap_geterr(pd));
    }
@@ -1599,7 +1665,7 @@
 #endif /* WIN32 */
 #endif /* HAVE_PCAP_LIB_VERSION */
    (void)fprintf(stderr,
-"Usage: %s [-aAd" D_FLAG "efKlLnNOpqRStu" U_FLAG "vxX]" B_FLAG_USAGE " [-c count] [ -C file_size ]\n", program_name);
+"Usage: %s [-aAbd" D_FLAG "efKlLnNOpqRStu" U_FLAG "vxX]" B_FLAG_USAGE " [-c count] [ -C file_size ]\n", program_name);
    (void)fprintf(stderr,
 "\t\t[ -E algo:secret ] [ -F file ] [ -G seconds ] [ -i interface ]\n");
    (void)fprintf(stderr,

[gvanem]

I don't object to the change, but now the main() function is in excess of 1000 lines. Can we move the getopt switch-statement to a new function? parse_cmdline().

BTW. Why is 'optarg' tested like this:

        case 'z':
          if (optarg) {
            zflag = strdup(optarg);
        } else {
            usage();
            /* NOTREACHED */
        }

getopt() is given a 'z:' which AFAIK makes 'optarg' always be present. At least in the 'getopt()' I'm used to.

[infrastation]

With regard to z:, you are right. How do you prefer to hand the commit over?
With regard to option parsing, do you have a patch?

[gvanem]

A patch for what exactly? Here is something to start with. My wish is to put minimal code inside the getopt() switch. So I've added a Dflag for option '-D' and a new function show_devices_and_exit(). Patch:

diff --git a/tcpdump.c b/tcpdump.c
index 1962789..47d66c4 100644
--- a/tcpdump.c
+++ b/tcpdump.c
@@ -98,6 +98,7 @@ extern int SIZE_BUF;
 netdissect_options Gndo;
 netdissect_options *gndo = &Gndo;

+static int Dflag;          /* list available devices and exit */
 static int dflag;          /* print filter code */
 static int Lflag;          /* list available data link types and exit */
 #ifdef HAVE_PCAP_SET_TSTAMP_TYPE
@@ -494,6 +495,29 @@ show_dlts_and_exit(const char *device, pcap_t *pd)
    exit(0);
 }

+#ifdef HAVE_PCAP_FINDALLDEVS
+static void
+show_devices_and_exit (void)
+
+   pcap_if_t *devpointer;
+
+   if (pcap_findalldevs(&devpointer, ebuf) < 0)
+       error("%s", ebuf);
+   else {
+       for (i = 0; devpointer != NULL; i++) {
+           printf("%d.%s", i+1, devpointer->name);
+           if (devpointer->description != NULL)
+               printf(" (%s)", devpointer->description);
+           if (devpointer->flags != 0)
+               printf(" [%s]", bittok2str(status_flags, "none", devpointer->flags));
+           printf("\n");
+           devpointer = devpointer->next;
+       }
+   }
+   exit(0);
+}
+#endif /* HAVE_PCAP_FINDALLDEVS */
+
 /*
  * Set up flags that might or might not be supported depending on the
  * version of libpcap we're using.
@@ -803,23 +827,9 @@ main(int argc, char **argv)
            ++dflag;
            break;

-#ifdef HAVE_PCAP_FINDALLDEVS
        case 'D':
-           if (pcap_findalldevs(&devpointer, ebuf) < 0)
-               error("%s", ebuf);
-           else {
-               for (i = 0; devpointer != NULL; i++) {
-                   printf("%d.%s", i+1, devpointer->name);
-                   if (devpointer->description != NULL)
-                       printf(" (%s)", devpointer->description);
-                   if (devpointer->flags != 0)
-                       printf(" [%s]", bittok2str(status_flags, "none", devpointer->flags));
-                   printf("\n");
-                   devpointer = devpointer->next;
-               }
-           }
-           return 0;
-#endif /* HAVE_PCAP_FINDALLDEVS */
+           Dflag++;
+           break;

        case 'L':
            Lflag++;
@@ -1131,22 +1141,11 @@ main(int argc, char **argv)
            break;
 #endif
        case 'z':
-           if (optarg) {
-               zflag = strdup(optarg);
-           } else {
-               usage();
-               /* NOTREACHED */
-           }
+           zflag = strdup(optarg);
            break;

        case 'Z':
-           if (optarg) {
-               username = strdup(optarg);
-           }
-           else {
-               usage();
-               /* NOTREACHED */
-           }
+           username = strdup(optarg);
            break;

        default:
@@ -1154,6 +1153,11 @@ main(int argc, char **argv)
            /* NOTREACHED */
        }

+#ifdef HAVE_PCAP_FINDALLDEVS
+   if (Dflag)
+       show_devices_and_exit();
+#endif
+
    switch (tflag) {

    case 0: /* Default */

We can probably cleanup case 'i' in a similar way.

[gvanem]

There should be a:

char ebuf[PCAP_ERRBUF_SIZE];

in there too.

[gvanem]

I cocked it up. Here is a patch that actually compiles:

diff --git a/tcpdump.c b/tcpdump.c
index 1962789..19d7f2b 100644
--- a/tcpdump.c
+++ b/tcpdump.c
@@ -98,6 +98,7 @@ extern int SIZE_BUF;
 netdissect_options Gndo;
 netdissect_options *gndo = &Gndo;

+static int Dflag;          /* list available devices and exit */
 static int dflag;          /* print filter code */
 static int Lflag;          /* list available data link types and exit */
 #ifdef HAVE_PCAP_SET_TSTAMP_TYPE
@@ -494,6 +495,31 @@ show_dlts_and_exit(const char *device, pcap_t *pd)
    exit(0);
 }

+#ifdef HAVE_PCAP_FINDALLDEVS
+static void
+show_devices_and_exit (void)
+{
+   pcap_if_t *devpointer;
+   char ebuf[PCAP_ERRBUF_SIZE];
+   int i;
+
+   if (pcap_findalldevs(&devpointer, ebuf) < 0)
+       error("%s", ebuf);
+   else {
+       for (i = 0; devpointer != NULL; i++) {
+           printf("%d.%s", i+1, devpointer->name);
+           if (devpointer->description != NULL)
+               printf(" (%s)", devpointer->description);
+           if (devpointer->flags != 0)
+               printf(" [%s]", bittok2str(status_flags, "none", devpointer->flags));
+           printf("\n");
+           devpointer = devpointer->next;
+       }
+   }
+   exit(0);
+}
+#endif /* HAVE_PCAP_FINDALLDEVS */
+
 /*
  * Set up flags that might or might not be supported depending on the
  * version of libpcap we're using.
@@ -803,23 +829,9 @@ main(int argc, char **argv)
            ++dflag;
            break;

-#ifdef HAVE_PCAP_FINDALLDEVS
        case 'D':
-           if (pcap_findalldevs(&devpointer, ebuf) < 0)
-               error("%s", ebuf);
-           else {
-               for (i = 0; devpointer != NULL; i++) {
-                   printf("%d.%s", i+1, devpointer->name);
-                   if (devpointer->description != NULL)
-                       printf(" (%s)", devpointer->description);
-                   if (devpointer->flags != 0)
-                       printf(" [%s]", bittok2str(status_flags, "none", devpointer->flags));
-                   printf("\n");
-                   devpointer = devpointer->next;
-               }
-           }
-           return 0;
-#endif /* HAVE_PCAP_FINDALLDEVS */
+           Dflag++;
+           break;

        case 'L':
            Lflag++;
@@ -1131,22 +1143,11 @@ main(int argc, char **argv)
            break;
 #endif
        case 'z':
-           if (optarg) {
-               zflag = strdup(optarg);
-           } else {
-               usage();
-               /* NOTREACHED */
-           }
+           zflag = strdup(optarg);
            break;

        case 'Z':
-           if (optarg) {
-               username = strdup(optarg);
-           }
-           else {
-               usage();
-               /* NOTREACHED */
-           }
+           username = strdup(optarg);
            break;

        default:
@@ -1154,6 +1155,11 @@ main(int argc, char **argv)
            /* NOTREACHED */
        }

+#ifdef HAVE_PCAP_FINDALLDEVS
+   if (Dflag)
+       show_devices_and_exit();
+#endif
+
    switch (tflag) {

    case 0: /* Default */

[infrastation]

Your patch is now commit 2808f56, thank you. For history: the check to MakeFilename() was independently made in commit b6fe309.