-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 07/16/2009 04:49 PM, John Dennis wrote: > This is a big patch, sorry, but there just isn't a realistic way to > develop a major piece of code into a working state given our checkin > policy which requires peer review for every change, it's just easier to > develop elsewhere and submit the working result. > > This is the basic framework for the client audit code, it produces an > executable called lwatch. In it's default mode lwatch watches a set of > (log) files. It understands how log files are rotated. When a log file > reaches a modification threshold it's newly appended data is prepared > for transport to the audit server. When the log file is rotated, > created, renamed, etc. the appropriate actions take place. > > lwatch maintains it's persistent state in a sqlite database. You can > start and stop lwatch and it self synchronizes based on what is in the > database and what it finds in the file system. > > lwatch is also capable of listing every log under a directory root, this > can be handy for initializing the set of log files to watch. > > lwatch can also dump in a pretty format the current state of the SQL > database. > > lwatch is not completely finished, but it's been in reasonable working > shape for a while now, but sitting in my own git repository, it's time > to get into the project's repository. What it needs next is better > configuration options and to link in the code for audit server > transmission (coming soon). > > Just one note about the patch, it includes a trivial one line fix to > dhash.h which was missing a const declaration. I realize that should > have been in a separate patch, but it got included here. > > > ------------------------------------------------------------------------ > > _______________________________________________ > Freeipa-devel mailing list > Freeipa-devel@redhat.com > https://www.redhat.com/mailman/listinfo/freeipa-devel
Review Part II lwatch/watch_database.c: sqlite3_result_string() - typo in the default case. "unkown" -> "unknown" table_exists() - You're looping through results to see if the table exists, but the SELECT query suggests that you should only be expecting a single result. If you get more than one result back, shouldn't that imply that the database has been corrupted and be treated as an error? populate_path_info() - misuse of strncpy(). Not ensuring null termination. You do this in many other places, so I will not repeat this complaint again. It would be prudent to have the positions of the values in the statement be #defines instead of magic numbers, to make it easier to change later if you add or remove columns. path_info_flags_string() - Same snprintf() misuse as I mentioned in my earlier review. Repeated in several other functions, so I will not repeat myself. util.c: pathname_table_string() - You are misusing realloc(). If realloc() returns NULL, you have leaked the original memory for tbl_string. regexp.c: Please prefer using the fully expanded error string PCRE_ERROR_* (as you do with the SQLite error strings in watch_database.c) inotify_watch.c: I don't see any additional problems other than those indicated above. This concludes my review of this iteration of the patch. - -- Stephen Gallagher RHCE 804006346421761 Looking to carve out IT costs? www.redhat.com/carveoutcosts/ -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkpotnUACgkQeiVVYja6o6MImgCfZf3qaMt+8wugE+WQTl5lKzYV f3IAnjzMmsLz+g0s8iwH8NhexE6yoeya =Utcx -----END PGP SIGNATURE----- _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel