[Snort-devel] Results of a quick/light code review

From: Steve G <linux_4ever(at)yahoo.com>
Date: Tue Sep 23 2003 - 11:30:50 EDT

Hello,

I'm continuing to go through the snort code and have some early results I'd like to share. The results admittedly border on cherry picking, but there are quite few things that need fixing.

If I have time, I'll try to get deeper into the code and do more review.

-Steve Grubb

---

Global comment:

• rindex, recv, sin, signal, time, free, read, write, system, index are bad variable names.

*foo(); is not an ANSI function prototype. foo(void); is. The
compiler treats them very differently.

*atoi is a deprecated function. strtol or strtoul should be used.

*errno should be cleared and checked after each use of strtol or
strtoul.

*bzero is a deprecated function. memset should be used.

---

spo_database.c:
1098: String format specified, ReferenceNode type was passed. sb refNode->system->name ?

spo_alert_sf_socket.c:
133: String format specified, no arguments 150: unsigned int format specified, long unsigned int passed

sp_icmp_type_check.c:
129: Several format specifiers in format string. No arguments passed.
138: Several format specifiers in format string. No arguments passed.

sp_ipoption_check.c:
131: Format string has no arguments specified. Two are passed.

sp_byte_check.c:
265: Character format specified, pointer passed.

spp_stream4.c:
999: Long unsigned int specified. int passed.

spp_perfmonitor.c:
291: 2 arguments specified by format, 3 passed

parser.c:
662: 2 arguments specified in format, only 1 is passed

1511: 4 arguments specified in format, only 3 are passed
3512: 3 arguments specified in format, only 1 is passed
3761: No arguments specified in format, 1 is passed

plugbase.c:
1230: The if statement has a ; to the right of the closing parenthesis

snort.c:
1708: I think there's an extra comma between format specifier strings.

signature.c:
224: 3 arguments specified in format, only 2 are passed 344: 3 arguments specified in format, only 1 is passed

sfthreshold.c:
348: Should mask be: 0x80000000 or 0xFFFFFFFF ? 1 is a signed number which has 31 bits, not 32. It then gets converted to unsigned. Using a constant is clearer.

---

Do you Yahoo!?
Yahoo! SiteBuilder - Free, easy-to-use web site design software http://sitebuilder.yahoo.com

---

This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven.
http://thinkgeek.com/sf

---

Snort-devel mailing list
Snort-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/snort-devel Received on Tue Sep 23 11:39:29 2003