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

Steve G linux_4ever at ...398...
Tue Sep 23 08:31:12 EDT 2003


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




More information about the Snort-devel mailing list