Skip to content

Commit c81dd51

Browse files
committed
DAG: Refine error handling in dag_findalldevs().
The only purpose of the dag_parse_name() invocation in this context is to derive "/dev/dagN" from "dagN", so replace it with snprintf() and eliminate an error condition to check. Add a check for the dag_rx_get_stream_count() invocation. Make sure to call dag_close() and fclose() even when failing with an error. fixup error handling
1 parent 850c8a6 commit c81dd51

File tree

1 file changed

+25
-20
lines changed

1 file changed

+25
-20
lines changed

pcap-dag.c

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1102,10 +1102,7 @@ dag_stream_long_description(const unsigned stream, const dag_size_t bufsize,
11021102
int
11031103
dag_findalldevs(pcap_if_list_t *devlistp, char *errbuf)
11041104
{
1105-
char name[DAGNAME_BUFSIZE];
11061105
int c;
1107-
char dagname[DAGNAME_BUFSIZE];
1108-
int dagstream;
11091106
int dagfd;
11101107
const char * description;
11111108
int stream, rxstreams;
@@ -1117,21 +1114,24 @@ dag_findalldevs(pcap_if_list_t *devlistp, char *errbuf)
11171114
// for "dag0:0"), thus the notion of link status does not apply to the
11181115
// resulting libpcap DAG capture devices.
11191116
const bpf_u_int32 flags = PCAP_IF_CONNECTION_STATUS_NOT_APPLICABLE;
1117+
FILE * sysfsinfo = NULL;
11201118

11211119
/* Try all the DAGs 0-DAG_MAX_BOARDS */
11221120
for (c = 0; c < DAG_MAX_BOARDS; c++) {
1121+
char name[DAGNAME_BUFSIZE]; // libpcap device
11231122
snprintf(name, sizeof(name), "dag%d", c);
1124-
if (-1 == dag_parse_name(name, dagname, DAGNAME_BUFSIZE, &dagstream))
1125-
{
1126-
(void) snprintf(errbuf, PCAP_ERRBUF_SIZE,
1127-
"dag: device name %s can't be parsed", name);
1128-
return PCAP_ERROR;
1129-
}
1123+
char dagname[DAGNAME_BUFSIZE]; // DAG API device
1124+
snprintf(dagname, sizeof(dagname), "/dev/dag%d", c);
11301125
if ( (dagfd = dag_open(dagname)) >= 0 ) {
11311126
// Do not add a shorthand device for stream 0 (dagN) yet -- the
11321127
// user can disable any stream in the card configuration.
11331128
const dag_card_inf_t * inf = dag_pciinfo(dagfd); // NULL is fine
11341129
rxstreams = dag_rx_get_stream_count(dagfd);
1130+
if (rxstreams < 0) {
1131+
pcapint_fmt_errmsg_for_errno(errbuf, PCAP_ERRBUF_SIZE,
1132+
errno, "dag_rx_get_stream_count");
1133+
goto failclose;
1134+
}
11351135
for(stream=0;stream<DAG_STREAM_MAX;stream+=2) {
11361136
if (0 == dag_attach_stream64(dagfd, stream, 0, 0)) {
11371137
// The Rx stream definitely exists and wasn't attached.
@@ -1148,16 +1148,13 @@ dag_findalldevs(pcap_if_list_t *devlistp, char *errbuf)
11481148
// a conditional shorthand device
11491149
if (stream == 0 &&
11501150
pcapint_add_dev(devlistp, name, flags, description, errbuf) == NULL)
1151-
return PCAP_ERROR;
1151+
goto failclose;
11521152
// and the stream device
11531153
snprintf(name, sizeof(name), "dag%d:%d", c, stream);
11541154
description = dag_stream_long_description(stream,
11551155
dag_get_stream_buffer_size64(dagfd, stream), inf);
11561156
if (pcapint_add_dev(devlistp, name, flags, description, errbuf) == NULL) {
1157-
/*
1158-
* Failure.
1159-
*/
1160-
return PCAP_ERROR;
1157+
goto failclose;
11611158
}
11621159

11631160
rxstreams--;
@@ -1167,6 +1164,7 @@ dag_findalldevs(pcap_if_list_t *devlistp, char *errbuf)
11671164
}
11681165
}
11691166
dag_close(dagfd);
1167+
dagfd = -1;
11701168
} else if (errno == EACCES) {
11711169
// The device exists, but the current user privileges are not
11721170
// sufficient for dag_open().
@@ -1176,29 +1174,36 @@ dag_findalldevs(pcap_if_list_t *devlistp, char *errbuf)
11761174
// memory.
11771175
char sysfspath[PATH_MAX];
11781176
snprintf(sysfspath, sizeof(sysfspath), "/sys/devices/virtual/dag/%s/info", name);
1179-
FILE *info = fopen(sysfspath, "r");
1180-
if (info) {
1177+
if ((sysfsinfo = fopen(sysfspath, "r"))) {
11811178
char linebuf[1024];
1182-
while (fgets(linebuf, sizeof(linebuf), info))
1179+
while (fgets(linebuf, sizeof(linebuf), sysfsinfo))
11831180
if (1 == sscanf(linebuf, "Stream %u:", &stream) && stream % 2 == 0) {
11841181
// a conditional shorthand device
11851182
description = dag_device_description(c);
11861183
if (stream == 0 &&
11871184
pcapint_add_dev(devlistp, name, flags, description, errbuf) == NULL)
1188-
return PCAP_ERROR;
1185+
goto failclose;
11891186
// and the stream device
11901187
snprintf(name, sizeof(name), "dag%u:%u", c, stream);
11911188
// TODO: Parse and describe the buffer size too.
11921189
description = dag_stream_short_description(stream);
11931190
if (pcapint_add_dev(devlistp, name, flags, description, errbuf) == NULL)
1194-
return PCAP_ERROR;
1191+
goto failclose;
11951192
}
1196-
fclose(info);
1193+
fclose(sysfsinfo);
1194+
sysfsinfo = NULL;
11971195
}
11981196
} // errno == EACCES
11991197

12001198
}
12011199
return (0);
1200+
1201+
failclose:
1202+
if (dagfd >= 0)
1203+
dag_close(dagfd);
1204+
if (sysfsinfo)
1205+
fclose(sysfsinfo);
1206+
return PCAP_ERROR;
12021207
}
12031208

12041209
static int

0 commit comments

Comments
 (0)