move assertion to correct place. Found by PVS-Studio
[freeradius.git] / src / main / detail.c
index 808cee5..3c0e95c 100644 (file)
@@ -80,11 +80,14 @@ int detail_send(rad_listen_t *listener, REQUEST *request)
                data->signal = 1;
                data->state = STATE_NO_REPLY;
 
-               RDEBUG("Detail - No response to request.  Will retry in %d seconds",
-                      data->retry_interval);
+               RDEBUG("detail (%s): No response to request.  Will retry in %d seconds",
+                      data->name, data->retry_interval);
        } else {
                int rtt;
                struct timeval now;
+
+               RDEBUG("detail (%s): Done %s packet.", data->name, fr_packet_codes[request->packet->code]);
+
                /*
                 *      We call gettimeofday a lot.  But it should be OK,
                 *      because there's nothing else to do.
@@ -152,8 +155,8 @@ int detail_send(rad_listen_t *listener, REQUEST *request)
                 */
                if (data->delay_time > (USEC / 4)) data->delay_time= USEC / 4;
 
-               RDEBUG3("Received response for request %d.  Will read the next packet in %d seconds",
-                       request->number, data->delay_time / USEC);
+               RDEBUG3("detail (%s): Received response for request %d.  Will read the next packet in %d seconds",
+                       data->name, request->number, data->delay_time / USEC);
 
                data->last_packet = now;
                data->signal = 1;
@@ -163,7 +166,7 @@ int detail_send(rad_listen_t *listener, REQUEST *request)
 
 #ifdef WITH_DETAIL_THREAD
        if (write(data->child_pipe[1], &c, 1) < 0) {
-               ERROR("Failed writing ack to reader thread: %s", fr_syserror(errno));
+               RERROR("detail (%s): Failed writing ack to reader thread: %s", data->name, fr_syserror(errno));
        }
 #else
        radius_signal_self(RADIUS_SIGNAL_SELF_DETAIL);
@@ -201,7 +204,22 @@ static int detail_open(rad_listen_t *this)
         *      this file will be read && processed before the
         *      file globbing is done.
         */
+       data->fp = NULL;
        data->work_fd = open(data->filename_work, O_RDWR);
+
+       /*
+        *      Couldn't open it for a reason OTHER than "it doesn't
+        *      exist".  Complain and tell the admin.
+        */
+       if ((data->work_fd < 0) && (errno != ENOENT)) {
+               ERROR("Failed opening detail file %s: %s",
+                     data->filename_work, fr_syserror(errno));
+               return 0;
+       }
+
+       /*
+        *      The file doesn't exist.  Poll for it again.
+        */
        if (data->work_fd < 0) {
 #ifndef HAVE_GLOB_H
                return 0;
@@ -212,7 +230,7 @@ static int detail_open(rad_listen_t *this)
                char const      *filename;
                glob_t          files;
 
-               DEBUG2("Polling for detail file %s", data->filename);
+               DEBUG2("detail (%s): Polling for detail file", data->name);
 
                memset(&files, 0, sizeof(files));
                if (glob(data->filename, 0, NULL, &files) != 0) {
@@ -243,10 +261,10 @@ static int detail_open(rad_listen_t *this)
                 */
                filename = files.gl_pathv[found];
 
-               DEBUG("Detail - Renaming %s -> %s", filename, data->filename_work);
+               DEBUG("detail (%s): Renaming %s -> %s", data->name, filename, data->filename_work);
                if (rename(filename, data->filename_work) < 0) {
-                       ERROR("Detail - Failed renaming %s to %s: %s",
-                             filename, data->filename_work, fr_syserror(errno));
+                       ERROR("detail (%s): Failed renaming %s to %s: %s",
+                             data->name, filename, data->filename_work, fr_syserror(errno));
                        goto noop;
                }
 
@@ -256,7 +274,11 @@ static int detail_open(rad_listen_t *this)
                 *      And try to open the filename.
                 */
                data->work_fd = open(data->filename_work, O_RDWR);
-               if (data->work_fd < 0) return 0;
+               if (data->work_fd < 0) {
+                       ERROR("Failed opening detail file %s: %s",
+                                       data->filename_work, fr_syserror(errno));
+                       return 0;
+               }
 #endif
        } /* else detail.work existed, and we opened it */
 
@@ -309,6 +331,18 @@ int detail_recv(rad_listen_t *listener)
        packet = detail_poll(listener);
        if (!packet) return -1;
 
+       if (DEBUG_ENABLED2) {
+               VALUE_PAIR *vp;
+               vp_cursor_t cursor;
+
+               DEBUG2("detail (%s): Read packet from %s", data->name, data->filename_work);
+               for (vp = fr_cursor_init(&cursor, &packet->vps);
+                    vp;
+                    vp = fr_cursor_next(&cursor)) {
+                       debug_pair(vp);
+               }
+       }
+
        switch (packet->code) {
        case PW_CODE_ACCOUNTING_REQUEST:
                fun = rad_accounting;
@@ -328,8 +362,7 @@ int detail_recv(rad_listen_t *listener)
        /*
         *      Don't bother doing limit checks, etc.
         */
-       if (!request_receive(NULL, listener, packet, &data->detail_client,
-                            fun)) {
+       if (!request_receive(NULL, listener, packet, &data->detail_client, fun)) {
                rad_free(&packet);
                data->state = STATE_NO_REPLY;   /* try again later */
                return 0;
@@ -354,6 +387,18 @@ int detail_recv(rad_listen_t *listener)
 
        rad_assert(packet != NULL);
 
+       if (DEBUG_ENABLED2) {
+               VALUE_PAIR *vp;
+               vp_cursor_t cursor;
+
+               DEBUG2("detail (%s): Read packet from %s", data->name, data->filename_work);
+               for (vp = fr_cursor_init(&cursor, &packet->vps);
+                    vp;
+                    vp = fr_cursor_next(&cursor)) {
+                       debug_pair(vp);
+               }
+       }
+
        switch (packet->code) {
        case PW_CODE_ACCOUNTING_REQUEST:
                fun = rad_accounting;
@@ -369,14 +414,14 @@ int detail_recv(rad_listen_t *listener)
                goto signal_thread;
        }
 
-       if (!request_receive(NULL, listener, packet, &data->detail_client,
-                            fun)) {
+       if (!request_receive(NULL, listener, packet, &data->detail_client, fun)) {
                data->state = STATE_NO_REPLY;   /* try again later */
 
        signal_thread:
                rad_free(&packet);
                if (write(data->child_pipe[1], &c, 1) < 0) {
-                       ERROR("Failed writing ack to reader thread: %s", fr_syserror(errno));
+                       ERROR("detail (%s): Failed writing ack to reader thread: %s", data->name,
+                             fr_syserror(errno));
                }
        }
 
@@ -389,6 +434,7 @@ int detail_recv(rad_listen_t *listener)
 
 static RADIUS_PACKET *detail_poll(rad_listen_t *listener)
 {
+       int             y;
        char            key[256], op[8], value[1024];
        vp_cursor_t     cursor;
        VALUE_PAIR      *vp;
@@ -408,12 +454,12 @@ open_file:
 
                /* FALL-THROUGH */
 
-               /*
-                *      Try to lock fd.  If we can't, return.
-                *      If we can, continue.  This means that
-                *      the server doesn't block while waiting
-                *      for the lock to open...
-                */
+       /*
+        *      Try to lock fd.  If we can't, return.
+        *      If we can, continue.  This means that
+        *      the server doesn't block while waiting
+        *      for the lock to open...
+        */
        case STATE_UNLOCKED:
                /*
                 *      Note that we do NOT block waiting for
@@ -433,15 +479,20 @@ open_file:
                         *      try again.
                         */
                        close(data->work_fd);
+                       data->fp = NULL;
                        data->work_fd = -1;
                        data->state = STATE_UNOPENED;
                        return NULL;
                }
 
-               data->fp = fdopen(data->work_fd, "r");
+               /*
+                *      Only open for writing if we're
+                *      marking requests as completed.
+                */
+               data->fp = fdopen(data->work_fd, data->track ? "r+" : "r");
                if (!data->fp) {
-                       ERROR("FATAL: Failed to re-open detail file %s: %s",
-                              data->filename, fr_syserror(errno));
+                       ERROR("detail (%s): FATAL: Failed to re-open detail file: %s",
+                             data->name, fr_syserror(errno));
                        fr_exit(1);
                }
 
@@ -469,10 +520,8 @@ open_file:
                        struct stat buf;
 
                        if (fstat(data->work_fd, &buf) < 0) {
-                               ERROR("Failed to stat "
-                                      "detail file %s: %s",
-                                       data->filename,
-                                       fr_syserror(errno));
+                               ERROR("detail (%s): Failed to stat detail file: %s",
+                                     data->name, fr_syserror(errno));
 
                                goto cleanup;
                        }
@@ -487,8 +536,7 @@ open_file:
                 */
                if (feof(data->fp)) {
                cleanup:
-                       DEBUG("Detail - unlinking %s",
-                             data->filename_work);
+                       DEBUG("detail (%s): Unlinking %s", data->name, data->filename_work);
                        unlink(data->filename_work);
                        if (data->fp) fclose(data->fp);
                        data->fp = NULL;
@@ -497,7 +545,7 @@ open_file:
                        rad_assert(data->vps == NULL);
 
                        if (data->one_shot) {
-                               INFO("Finished reading \"one shot\" detail file - Exiting");
+                               INFO("detail (%s): Finished reading \"one shot\" detail file - Exiting", data->name);
                                radius_signal_self(RADIUS_SIGNAL_SELF_EXIT);
                        }
 
@@ -507,205 +555,237 @@ open_file:
                /*
                 *      Else go read something.
                 */
-               break;
+               if (!fgets(buffer, sizeof(buffer), data->fp)) {
+                       DEBUG("detail (%s): Failed reading header from file - %s",
+                             data->name, data->filename_work);
+                       goto cleanup;
+               }
 
                /*
-                *      Read more value-pair's, unless we're
-                *      at EOF.  In that case, queue whatever
-                *      we have.
+                *      Badly formatted file: delete it.
                 */
-       case STATE_READING:
-               if (data->fp && !feof(data->fp)) break;
-               data->state = STATE_QUEUED;
+               if (!strchr(buffer, '\n')) {
+                       DEBUG("detail (%s): Invalid line without trailing LF - %s", data->name, buffer);
+                       goto cleanup;
+               }
+
+               if (!sscanf(buffer, "%*s %*s %*d %*d:%*d:%*d %d", &y)) {
+                       DEBUG("detail (%s): Failed reading detail file header in line - %s", data->name, buffer);
+                       goto cleanup;
+               }
 
+               data->state = STATE_READING;
                /* FALL-THROUGH */
 
-       case STATE_QUEUED:
-               goto alloc_packet;
 
-               /*
-                *      Periodically check what's going on.
-                *      If the request is taking too long,
-                *      retry it.
-                */
-       case STATE_RUNNING:
-               if (time(NULL) < (data->running + (int)data->retry_interval)) {
-                       return NULL;
-               }
+       /*
+        *      Read more value-pair's, unless we're
+        *      at EOF.  In that case, queue whatever
+        *      we have.
+        */
+       case STATE_READING:
+               rad_assert(data->fp != NULL);
 
-               DEBUG("No response to detail request.  Retrying");
-               /* FALL-THROUGH */
+               fr_cursor_init(&cursor, &data->vps);
 
                /*
-                *      If there's no reply, keep
-                *      retransmitting the current packet
-                *      forever.
+                *      Read a header, OR a value-pair.
                 */
-       case STATE_NO_REPLY:
-               data->state = STATE_QUEUED;
-               goto alloc_packet;
+               while (fgets(buffer, sizeof(buffer), data->fp)) {
+                       data->last_offset = data->offset;
+                       data->offset = ftell(data->fp); /* for statistics */
 
-               /*
-                *      We have a reply.  Clean up the old
-                *      request, and go read another one.
-                */
-       case STATE_REPLIED:
-               if (data->track) {
-                       rad_assert(data->fp != NULL);
+                       /*
+                        *      Badly formatted file: delete it.
+                        */
+                       if (!strchr(buffer, '\n')) {
+                               WARN("detail (%s): Skipping line without trailing LF - %s", data->name, buffer);
+                               fr_pair_list_free(&data->vps);
+                               goto cleanup;
+                       }
 
-                       if ((fseek(data->fp, data->timestamp_offset, SEEK_SET) < 0) ||
-                           (fwrite("\tDone", 1, 5, data->fp) < 5)) {
-                               WARN("Failed marking detail request as done: %s", fr_syserror(errno));
+                       /*
+                        *      We're reading VP's, and got a blank line.
+                        *      That indicates the end of an entry.  Queue the
+                        *      packet.
+                        */
+                       if (buffer[0] == '\n') {
+                               data->state = STATE_QUEUED;
+                               data->tries = 0;
+                               data->packets++;
+                               goto alloc_packet;
                        }
-                       fflush(data->fp);
-                       if (fseek(data->fp, data->offset, SEEK_SET) < 0) {
-                               WARN("Failed seeking to next detail request: %s", fr_syserror(errno));
+
+                       /*
+                        *      We have a full "attribute = value" line.
+                        *      If it doesn't look reasonable, skip it.
+                        *
+                        *      FIXME: print an error for badly formatted attributes?
+                        */
+                       if (sscanf(buffer, "%255s %7s %1023s", key, op, value) != 3) {
+                               DEBUG("detail (%s): Skipping badly formatted line - %s", data->name, buffer);
+                               continue;
                        }
-               }
 
-               pairfree(&data->vps);
-               data->state = STATE_HEADER;
-               goto do_header;
-       }
+                       /*
+                        *      Should be =, :=, +=, ...
+                        */
+                       if (!strchr(op, '=')) {
+                               DEBUG("detail (%s): Skipping line without operator - %s", data->name, buffer);
+                               continue;
+                       }
 
-       fr_cursor_init(&cursor, &data->vps);
+                       /*
+                        *      Skip non-protocol attributes.
+                        */
+                       if (!strcasecmp(key, "Request-Authenticator")) continue;
 
-       /*
-        *      Read a header, OR a value-pair.
-        */
-       while (fgets(buffer, sizeof(buffer), data->fp)) {
-               data->last_offset = data->offset;
-               data->offset = ftell(data->fp); /* for statistics */
+                       /*
+                        *      Set the original client IP address, based on
+                        *      what's in the detail file.
+                        *
+                        *      Hmm... we don't set the server IP address.
+                        *      or port.  Oh well.
+                        */
+                       if (!strcasecmp(key, "Client-IP-Address")) {
+                               data->client_ip.af = AF_INET;
+                               if (ip_hton(&data->client_ip, AF_INET, value, false) < 0) {
+                                       DEBUG("detail (%s): Failed parsing Client-IP-Address", data->name);
+                                       fr_pair_list_free(&data->vps);
+                                       goto cleanup;
+                               }
+                               continue;
+                       }
 
-               /*
-                *      Badly formatted file: delete it.
-                *
-                *      FIXME: Maybe flag an error?
-                */
-               if (!strchr(buffer, '\n')) {
-                       pairfree(&data->vps);
-                       goto cleanup;
-               }
+                       /*
+                        *      The original time at which we received the
+                        *      packet.  We need this to properly calculate
+                        *      Acct-Delay-Time.
+                        */
+                       if (!strcasecmp(key, "Timestamp")) {
+                               data->timestamp = atoi(value);
+                               data->timestamp_offset = data->last_offset;
+
+                               vp = fr_pair_afrom_num(data, PW_PACKET_ORIGINAL_TIMESTAMP, 0);
+                               if (vp) {
+                                       vp->vp_date = (uint32_t) data->timestamp;
+                                       vp->type = VT_DATA;
+                                       fr_cursor_insert(&cursor, vp);
+                               }
+                               continue;
+                       }
 
-               /*
-                *      We're reading VP's, and got a blank line.
-                *      Queue the packet.
-                */
-               if ((data->state == STATE_READING) &&
-                   (buffer[0] == '\n')) {
-                       data->state = STATE_QUEUED;
-                       break;
-               }
+                       if (!strcasecmp(key, "Donestamp")) {
+                               data->timestamp = atoi(value);
+                               data->done_entry = true;
+                               continue;
+                       }
 
-               /*
-                *      Look for date/time header, and read VP's if
-                *      found.  If not, keep reading lines until we
-                *      find one.
-                */
-               if (data->state == STATE_HEADER) {
-                       int y;
+                       DEBUG3("detail (%s): Trying to read VP from line - %s", data->name, buffer);
 
-                       if (sscanf(buffer, "%*s %*s %*d %*d:%*d:%*d %d", &y)) {
-                               data->state = STATE_READING;
+                       /*
+                        *      Read one VP.
+                        *
+                        *      FIXME: do we want to check for non-protocol
+                        *      attributes like radsqlrelay does?
+                        */
+                       vp = NULL;
+                       if ((fr_pair_list_afrom_str(data, buffer, &vp) > 0) &&
+                           (vp != NULL)) {
+                               fr_cursor_merge(&cursor, vp);
+                       } else {
+                               DEBUG("detail (%s): Failed reading VP from line - %s", data->name, buffer);
+                               goto cleanup;
                        }
-                       continue;
                }
 
                /*
-                *      We have a full "attribute = value" line.
-                *      If it doesn't look reasonable, skip it.
-                *
-                *      FIXME: print an error for badly formatted attributes?
+                *      The writer doesn't check that the
+                *      record was completely written.  If the
+                *      disk is full, this can result in a
+                *      truncated record which has no trailing
+                *      blank line.  When that happens, it's a
+                *      bad record, and we ignore it.
                 */
-               if (sscanf(buffer, "%255s %7s %1023s", key, op, value) != 3) {
-                       WARN("Skipping badly formatted line %s",
-                              buffer);
-                       continue;
+               if (feof(data->fp)) {
+                       DEBUG("detail (%s): Truncated record: treating it as EOF for detail file %s",
+                             data->name, data->filename_work);
+                       fr_pair_list_free(&data->vps);
+                       goto cleanup;
                }
 
                /*
-                *      Should be =, :=, +=, ...
-                */
-               if (!strchr(op, '=')) continue;
-
-               /*
-                *      Skip non-protocol attributes.
-                */
-               if (!strcasecmp(key, "Request-Authenticator")) continue;
-
-               /*
-                *      Set the original client IP address, based on
-                *      what's in the detail file.
+                *      Some kind of non-eof error.
                 *
-                *      Hmm... we don't set the server IP address.
-                *      or port.  Oh well.
+                *      FIXME: Leave the file in-place, and warn the
+                *      administrator?
                 */
-               if (!strcasecmp(key, "Client-IP-Address")) {
-                       data->client_ip.af = AF_INET;
-                       if (ip_hton(&data->client_ip, AF_INET, value, false) < 0) {
-                               ERROR("Failed parsing Client-IP-Address");
+               DEBUG("detail (%s): Unknown error, deleting detail file %s",
+                     data->name, data->filename_work);
+               goto cleanup;
 
-                               pairfree(&data->vps);
-                               goto cleanup;
-                       }
-                       continue;
-               }
+       case STATE_QUEUED:
+               goto alloc_packet;
 
-               /*
-                *      The original time at which we received the
-                *      packet.  We need this to properly calculate
-                *      Acct-Delay-Time.
-                */
-               if (!strcasecmp(key, "Timestamp")) {
-                       data->timestamp = atoi(value);
-                       data->timestamp_offset = data->last_offset;
-
-                       vp = paircreate(data, PW_PACKET_ORIGINAL_TIMESTAMP, 0);
-                       if (vp) {
-                               vp->vp_date = (uint32_t) data->timestamp;
-                               vp->type = VT_DATA;
-                               fr_cursor_insert(&cursor, vp);
-                       }
-                       continue;
+       /*
+        *      Periodically check what's going on.
+        *      If the request is taking too long,
+        *      retry it.
+        */
+       case STATE_RUNNING:
+               if (time(NULL) < (data->running + (int)data->retry_interval)) {
+                       return NULL;
                }
 
-               if (!strcasecmp(key, "Donestamp")) {
-                       data->timestamp = atoi(value);
-                       data->done_entry = true;
-                       continue;
-               }
+               DEBUG("detail (%s): No response to detail request.  Retrying", data->name);
+               /* FALL-THROUGH */
 
-               /*
-                *      Read one VP.
-                *
-                *      FIXME: do we want to check for non-protocol
-                *      attributes like radsqlrelay does?
-                */
-               vp = NULL;
-               if ((userparse(data, buffer, &vp) > 0) &&
-                   (vp != NULL)) {
-                       fr_cursor_merge(&cursor, vp);
-               }
-       }
+       /*
+        *      If there's no reply, keep
+        *      retransmitting the current packet
+        *      forever.
+        */
+       case STATE_NO_REPLY:
+               data->state = STATE_QUEUED;
+               goto alloc_packet;
 
        /*
-        *      Some kind of error.
-        *
-        *      FIXME: Leave the file in-place, and warn the
-        *      administrator?
+        *      We have a reply.  Clean up the old
+        *      request, and go read another one.
         */
-       if (ferror(data->fp)) goto cleanup;
+       case STATE_REPLIED:
+               if (data->track) {
+                       rad_assert(data->fp != NULL);
 
-       data->tries = 0;
-       data->packets++;
+                       if (fseek(data->fp, data->timestamp_offset, SEEK_SET) < 0) {
+                               DEBUG("detail (%s): Failed seeking to timestamp offset: %s",
+                                    data->name, fr_syserror(errno));
+                       } else if (fwrite("\tDone", 1, 5, data->fp) < 5) {
+                               DEBUG("detail (%s): Failed marking request as done: %s",
+                                    data->name, fr_syserror(errno));
+                       } else if (fflush(data->fp) != 0) {
+                               DEBUG("detail (%s): Failed flushing marked detail file to disk: %s",
+                                    data->name, fr_syserror(errno));
+                       }
+
+                       if (fseek(data->fp, data->offset, SEEK_SET) < 0) {
+                               DEBUG("detail (%s): Failed seeking to next detail request: %s",
+                                    data->name, fr_syserror(errno));
+                       }
+               }
+
+               fr_pair_list_free(&data->vps);
+               data->state = STATE_HEADER;
+               goto do_header;
+       }
 
        /*
         *      Process the packet.
         */
  alloc_packet:
        if (data->done_entry) {
-               DEBUG2("Skipping record for timestamp %lu", data->timestamp);
-               pairfree(&data->vps);
+               DEBUG2("detail (%s): Skipping record for timestamp %lu", data->name, data->timestamp);
+               fr_pair_list_free(&data->vps);
                data->state = STATE_HEADER;
                goto do_header;
        }
@@ -713,23 +793,13 @@ open_file:
        data->tries++;
 
        /*
-        *      The writer doesn't check that the record was
-        *      completely written.  If the disk is full, this can
-        *      result in a truncated record.  When that happens,
-        *      treat it as EOF.
-        */
-       if (data->state != STATE_QUEUED) {
-               ERROR("Truncated record: treating it as EOF for detail file %s", data->filename_work);
-               goto cleanup;
-       }
-
-       /*
         *      We're done reading the file, but we didn't read
         *      anything.  Clean up, and don't return anything.
         */
        if (!data->vps) {
+               WARN("detail (%s): Read empty packet from file %s",
+                    data->name, data->filename_work);
                data->state = STATE_HEADER;
-               if (!data->fp || feof(data->fp)) goto cleanup;
                return NULL;
        }
 
@@ -739,7 +809,7 @@ open_file:
         */
        packet = rad_alloc(NULL, true);
        if (!packet) {
-               ERROR("FATAL: Failed allocating memory for detail");
+               ERROR("detail (%s): FATAL: Failed allocating memory for detail", data->name);
                fr_exit(1);
        }
 
@@ -753,10 +823,10 @@ open_file:
         *      Otherwise, it lets us re-send the original packet
         *      contents, unmolested.
         */
-       packet->vps = paircopy(packet, data->vps);
+       packet->vps = fr_pair_list_copy(packet, data->vps);
 
        packet->code = PW_CODE_ACCOUNTING_REQUEST;
-       vp = pairfind(packet->vps, PW_PACKET_TYPE, 0, TAG_ANY);
+       vp = fr_pair_find_by_num(packet->vps, PW_PACKET_TYPE, 0, TAG_ANY);
        if (vp) packet->code = vp->vp_integer;
 
        gettimeofday(&packet->timestamp, NULL);
@@ -769,13 +839,13 @@ open_file:
                packet->src_ipaddr = data->client_ip;
        }
 
-       vp = pairfind(packet->vps, PW_PACKET_SRC_IP_ADDRESS, 0, TAG_ANY);
+       vp = fr_pair_find_by_num(packet->vps, PW_PACKET_SRC_IP_ADDRESS, 0, TAG_ANY);
        if (vp) {
                packet->src_ipaddr.af = AF_INET;
                packet->src_ipaddr.ipaddr.ip4addr.s_addr = vp->vp_ipaddr;
                packet->src_ipaddr.prefix = 32;
        } else {
-               vp = pairfind(packet->vps, PW_PACKET_SRC_IPV6_ADDRESS, 0, TAG_ANY);
+               vp = fr_pair_find_by_num(packet->vps, PW_PACKET_SRC_IPV6_ADDRESS, 0, TAG_ANY);
                if (vp) {
                        packet->src_ipaddr.af = AF_INET6;
                        memcpy(&packet->src_ipaddr.ipaddr.ip6addr,
@@ -784,13 +854,13 @@ open_file:
                }
        }
 
-       vp = pairfind(packet->vps, PW_PACKET_DST_IP_ADDRESS, 0, TAG_ANY);
+       vp = fr_pair_find_by_num(packet->vps, PW_PACKET_DST_IP_ADDRESS, 0, TAG_ANY);
        if (vp) {
                packet->dst_ipaddr.af = AF_INET;
                packet->dst_ipaddr.ipaddr.ip4addr.s_addr = vp->vp_ipaddr;
                packet->dst_ipaddr.prefix = 32;
        } else {
-               vp = pairfind(packet->vps, PW_PACKET_DST_IPV6_ADDRESS, 0, TAG_ANY);
+               vp = fr_pair_find_by_num(packet->vps, PW_PACKET_DST_IPV6_ADDRESS, 0, TAG_ANY);
                if (vp) {
                        packet->dst_ipaddr.af = AF_INET6;
                        memcpy(&packet->dst_ipaddr.ipaddr.ip6addr,
@@ -819,7 +889,7 @@ open_file:
                 *      "Timestamp" field is when we wrote the packet to the
                 *      detail file, which could have been much later.
                 */
-               vp = pairfind(packet->vps, PW_EVENT_TIMESTAMP, 0, TAG_ANY);
+               vp = fr_pair_find_by_num(packet->vps, PW_EVENT_TIMESTAMP, 0, TAG_ANY);
                if (vp) {
                        data->timestamp = vp->vp_integer;
                }
@@ -828,11 +898,11 @@ open_file:
                 *      Look for Acct-Delay-Time, and update
                 *      based on Acct-Delay-Time += (time(NULL) - timestamp)
                 */
-               vp = pairfind(packet->vps, PW_ACCT_DELAY_TIME, 0, TAG_ANY);
+               vp = fr_pair_find_by_num(packet->vps, PW_ACCT_DELAY_TIME, 0, TAG_ANY);
                if (!vp) {
-                       vp = paircreate(packet, PW_ACCT_DELAY_TIME, 0);
+                       vp = fr_pair_afrom_num(packet, PW_ACCT_DELAY_TIME, 0);
                        rad_assert(vp != NULL);
-                       pairadd(&packet->vps, vp);
+                       fr_pair_add(&packet->vps, vp);
                }
                if (data->timestamp != 0) {
                        vp->vp_integer += time(NULL) - data->timestamp;
@@ -842,23 +912,14 @@ open_file:
        /*
         *      Set the transmission count.
         */
-       vp = pairfind(packet->vps, PW_PACKET_TRANSMIT_COUNTER, 0, TAG_ANY);
+       vp = fr_pair_find_by_num(packet->vps, PW_PACKET_TRANSMIT_COUNTER, 0, TAG_ANY);
        if (!vp) {
-               vp = paircreate(packet, PW_PACKET_TRANSMIT_COUNTER, 0);
+               vp = fr_pair_afrom_num(packet, PW_PACKET_TRANSMIT_COUNTER, 0);
                rad_assert(vp != NULL);
-               pairadd(&packet->vps, vp);
+               fr_pair_add(&packet->vps, vp);
        }
        vp->vp_integer = data->tries;
 
-       if (debug_flag) {
-               fr_printf_log("detail_recv: Read packet from %s\n", data->filename_work);
-               for (vp = fr_cursor_init(&cursor, &packet->vps);
-                    vp;
-                    vp = fr_cursor_next(&cursor)) {
-                       debug_pair(vp);
-               }
-       }
-
        data->state = STATE_RUNNING;
        data->running = packet->timestamp.tv_sec;
 
@@ -885,7 +946,7 @@ void detail_free(rad_listen_t *this)
                data->child_pipe[0] = -1;
 
                /*
-                *      Tell it to stop (interrupting it's sleep)
+                *      Tell it to stop (interrupting its sleep)
                 */
                pthread_kill(data->pthread_id, SIGTERM);
 
@@ -894,10 +955,12 @@ void detail_free(rad_listen_t *this)
                 */
                ret = read(data->master_pipe[0], &arg, sizeof(arg));
                if (ret < 0) {
-                       ERROR("Reader thread exited without informing the master: %s", fr_syserror(errno));
+                       ERROR("detail (%s): Reader thread exited without informing the master: %s",
+                             data->name, fr_syserror(errno));
                } else if (ret != sizeof(arg)) {
-                       ERROR("Invalid thread pointer received from reader thread during exit");
-                       ERROR("Expected %zu bytes, got %zi bytes", sizeof(arg), ret);
+                       ERROR("detail (%s): Invalid thread pointer received from reader thread during exit",
+                             data->name);
+                       ERROR("detail (%s): Expected %zu bytes, got %zi bytes", data->name, sizeof(arg), ret);
                }
 
                close(data->master_pipe[0]);
@@ -940,8 +1003,8 @@ static int detail_delay(listen_detail_t *data)
        delay += (USEC * 3) / 4;
        delay += fr_rand() % (USEC / 2);
 
-       DEBUG2("Detail listener %s state %s waiting %d.%06d sec",
-              data->filename,
+       DEBUG2("detail (%s): Detail listener state %s waiting %d.%06d sec",
+              data->name,
               fr_int2str(state_names, data->state, "?"),
               (delay / USEC), delay % USEC);
 
@@ -965,8 +1028,9 @@ int detail_encode(UNUSED rad_listen_t *this, UNUSED REQUEST *request)
 
        data->signal = 0;
 
-       DEBUG2("Detail listener %s state %s signalled %d waiting %d.%06d sec",
-              data->filename, fr_int2str(state_names, data->state, "?"),
+       DEBUG2("detail (%s): Detail listener state %s signalled %d waiting %d.%06d sec",
+              data->name,
+              fr_int2str(state_names, data->state, "?"),
               data->signal,
               data->delay_time / USEC,
               data->delay_time % USEC);
@@ -978,13 +1042,25 @@ int detail_encode(UNUSED rad_listen_t *this, UNUSED REQUEST *request)
 /*
  *     Overloaded to return "should we fix delay times"
  */
-int detail_decode(UNUSED rad_listen_t *this, UNUSED REQUEST *request)
+int detail_decode(rad_listen_t *this, REQUEST *request)
 {
 #ifdef WITH_DETAIL_THREAD
+       listen_detail_t *data = this->data;
+
+       RDEBUG("Received %s from detail file %s",
+              fr_packet_codes[request->packet->code], data->filename_work);
+
+       rdebug_pair_list(L_DBG_LVL_1, request, request->packet->vps, "\t");
+
        return 0;
 #else
        listen_detail_t *data = this->data;
 
+       RDEBUG("Received %s from detail file %s",
+              fr_packet_codes[request->packet->code], data->filename_work);
+
+       rdebug_pair_list(L_DBG_LVL_1, request, request->packet->vps, "\t");
+
        return data->signal;
 #endif
 }
@@ -1010,7 +1086,8 @@ static void *detail_handler_thread(void *arg)
                        if (data->child_pipe[0] < 0) {
                                packet = NULL;
                                if (write(data->master_pipe[1], &packet, sizeof(packet)) < 0) {
-                                       ERROR("Failed writing exit status to master: %s", fr_syserror(errno));
+                                       ERROR("detail (%s): Failed writing exit status to master: %s",
+                                             data->name, fr_syserror(errno));
                                }
                                return NULL;
                        }
@@ -1023,11 +1100,13 @@ static void *detail_handler_thread(void *arg)
                 */
                do {
                        if (write(data->master_pipe[1], &packet, sizeof(packet)) < 0) {
-                               ERROR("Failed passing detail packet pointer to master: %s", fr_syserror(errno));
+                               ERROR("detail (%s): Failed passing detail packet pointer to master: %s",
+                                     data->name, fr_syserror(errno));
                        }
 
                        if (read(data->child_pipe[0], &c, 1) < 0) {
-                               ERROR("Failed getting detail packet ack from master: %s", fr_syserror(errno));
+                               ERROR("detail (%s): Failed getting detail packet ack from master: %s",
+                                     data->name, fr_syserror(errno));
                                break;
                        }
 
@@ -1049,11 +1128,9 @@ static const CONF_PARSER detail_config[] = {
        { "load_factor", FR_CONF_OFFSET(PW_TYPE_INTEGER, listen_detail_t, load_factor), STRINGIFY(10) },
        { "poll_interval", FR_CONF_OFFSET(PW_TYPE_INTEGER, listen_detail_t, poll_interval), STRINGIFY(1) },
        { "retry_interval", FR_CONF_OFFSET(PW_TYPE_INTEGER, listen_detail_t, retry_interval), STRINGIFY(30) },
-       { "one_shot", FR_CONF_OFFSET(PW_TYPE_BOOLEAN, listen_detail_t, one_shot), NULL },
-       { "track", FR_CONF_OFFSET(PW_TYPE_BOOLEAN, listen_detail_t, track), NULL },
-       { "max_outstanding", FR_CONF_OFFSET(PW_TYPE_INTEGER, listen_detail_t, load_factor), NULL },
-
-       { NULL, -1, 0, NULL, NULL }             /* end the list */
+       { "one_shot", FR_CONF_OFFSET(PW_TYPE_BOOLEAN, listen_detail_t, one_shot), "no" },
+       { "track", FR_CONF_OFFSET(PW_TYPE_BOOLEAN, listen_detail_t, track), "no" },
+       CONF_PARSER_TERMINATOR
 };
 
 /*
@@ -1064,7 +1141,7 @@ int detail_parse(CONF_SECTION *cs, rad_listen_t *this)
        int             rcode;
        listen_detail_t *data;
        RADCLIENT       *client;
-       char buffer[2048];
+       char            buffer[2048];
 
        data = this->data;
 
@@ -1074,6 +1151,9 @@ int detail_parse(CONF_SECTION *cs, rad_listen_t *this)
                return -1;
        }
 
+       data->name = cf_section_name2(cs);
+       if (!data->name) data->name = data->filename;
+
        /*
         *      We don't do duplicate detection for "detail" sockets.
         */
@@ -1085,24 +1165,21 @@ int detail_parse(CONF_SECTION *cs, rad_listen_t *this)
                return -1;
        }
 
-       if ((data->load_factor < 1) || (data->load_factor > 100)) {
-               cf_log_err_cs(cs, "Load factor must be between 1 and 100");
-               return -1;
-       }
+       FR_INTEGER_BOUND_CHECK("load_factor", data->load_factor, >=, 1);
+       FR_INTEGER_BOUND_CHECK("load_factor", data->load_factor, <=, 100);
 
-       if ((data->poll_interval < 1) || (data->poll_interval > 20)) {
-               cf_log_err_cs(cs, "poll_interval must be between 1 and 20");
-               return -1;
-       }
-
-       if (check_config) return 0;
-
-       if (data->max_outstanding == 0) data->max_outstanding = 1;
+       FR_INTEGER_BOUND_CHECK("poll_interval", data->poll_interval, >=, 1);
+       FR_INTEGER_BOUND_CHECK("poll_interval", data->poll_interval, <=, 60);
 
        FR_INTEGER_BOUND_CHECK("retry_interval", data->retry_interval, >=, 4);
        FR_INTEGER_BOUND_CHECK("retry_interval", data->retry_interval, <=, 3600);
 
        /*
+        *      Only checking the config.  Don't start threads or anything else.
+        */
+       if (check_config) return 0;
+
+       /*
         *      If the filename is a glob, use "detail.work" as the
         *      work file name.
         */
@@ -1111,8 +1188,8 @@ int detail_parse(CONF_SECTION *cs, rad_listen_t *this)
                char *p;
 
 #ifndef HAVE_GLOB_H
-               WARN("Detail file \"%s\" appears to use file globbing, but it is not supported on this system.",
-                    data->filename);
+               WARN("detail (%s): File \"%s\" appears to use file globbing, but it is not supported on this system",
+                    data->name, data->filename);
 #endif
                strlcpy(buffer, data->filename, sizeof(buffer));
                p = strrchr(buffer, FR_DIR_SEP);
@@ -1121,6 +1198,16 @@ int detail_parse(CONF_SECTION *cs, rad_listen_t *this)
                } else {
                        buffer[0] = '\0';
                }
+
+               /*
+                *      Globbing cannot be done across directories.
+                */
+               if ((strchr(buffer, '*') != NULL) ||
+                   (strchr(buffer, '[') != NULL)) {
+                       cf_log_err_cs(cs, "Wildcard directories are not supported");
+                       return -1;
+               }
+
                strlcat(buffer, "detail.work",
                        sizeof(buffer) - strlen(buffer));
 
@@ -1154,14 +1241,12 @@ int detail_parse(CONF_SECTION *cs, rad_listen_t *this)
         *      Create the communication pipes.
         */
        if (pipe(data->master_pipe) < 0) {
-               ERROR("radiusd: Error opening internal pipe: %s",
-                     fr_syserror(errno));
+               ERROR("detail (%s): Error opening internal pipe: %s", data->name, fr_syserror(errno));
                fr_exit(1);
        }
 
        if (pipe(data->child_pipe) < 0) {
-               ERROR("radiusd: Error opening internal pipe: %s",
-                     fr_syserror(errno));
+               ERROR("detail (%s): Error opening internal pipe: %s", data->name, fr_syserror(errno));
                fr_exit(1);
        }