checks the mtime of /var/log/messages and don't read it if it didn't change since last scan.
Please review.
J.
On Fri, 2010-03-12 at 17:52 +0100, Jiri Moskovcak wrote:
checks the mtime of /var/log/messages and don't read it if it didn't change since last scan.
The mtime is logically attached to a specific file. Using a static variable for it would make it impossible to figure out to which file this mtime belongs.
I suggest adding a "time_t *mtime_p" parameter to scan_syslog_file() which points to a place where old mtime may be found, and new one will be stored on return.
The parameter can be NULL, which means "do not perform scan avoidance" - for example, dumpoops will pass NULL.
CKerneloopsScanner::Run() will pass an address of class member variable.
- if (fstat(fd, &statb) != 0 || statb.st_size < 1) { + if (fstat(fd, &statb) != 0 || statb.st_size < 1) + { close(fd); return 0; } + if (last_changed == statb.st_mtime) + { + VERB1 log("Syslog file '%s' haven't change since last scan, skipping", filename); + close(fd); + return 0; + } + last_changed = statb.st_mtime;
Captain Nitpicker reports: style deviation detected. This file uses kernel-style brace placement and indent-by-tab. The changes do not.
On 03/12/2010 06:04 PM, Denys Vlasenko wrote:
On Fri, 2010-03-12 at 17:52 +0100, Jiri Moskovcak wrote:
checks the mtime of /var/log/messages and don't read it if it didn't change since last scan.
The mtime is logically attached to a specific file. Using a static variable for it would make it impossible to figure out to which file this mtime belongs.
I suggest adding a "time_t *mtime_p" parameter to scan_syslog_file() which points to a place where old mtime may be found, and new one will be stored on return.
The parameter can be NULL, which means "do not perform scan avoidance" - for example, dumpoops will pass NULL.
Makes sense, so I changed the patch and here it is, please review.
J.
CKerneloopsScanner::Run() will pass an address of class member variable.
if (fstat(fd,&statb) != 0 || statb.st_size< 1) {
if (fstat(fd,&statb) != 0 || statb.st_size< 1)
{ close(fd); return 0; }
if (last_changed == statb.st_mtime)
{
VERB1 log("Syslog file '%s' haven't change since last scan, skipping", filename);
close(fd);
return 0;
}
last_changed = statb.st_mtime;
Captain Nitpicker reports: style deviation detected. This file uses kernel-style brace placement and indent-by-tab. The changes do not.
On Mon, 2010-03-15 at 14:08 +0100, Jiri Moskovcak wrote:
On 03/12/2010 06:04 PM, Denys Vlasenko wrote:
On Fri, 2010-03-12 at 17:52 +0100, Jiri Moskovcak wrote:
checks the mtime of /var/log/messages and don't read it if it didn't change since last scan.
The mtime is logically attached to a specific file. Using a static variable for it would make it impossible to figure out to which file this mtime belongs.
I suggest adding a "time_t *mtime_p" parameter to scan_syslog_file() which points to a place where old mtime may be found, and new one will be stored on return.
The parameter can be NULL, which means "do not perform scan avoidance" - for example, dumpoops will pass NULL.
Makes sense, so I changed the patch and here it is, please review.
Looks good to me. I also compile-tested it - build was ok.
Pushed, thx.
J.
On 03/16/2010 02:35 PM, Denys Vlasenko wrote:
On Mon, 2010-03-15 at 14:08 +0100, Jiri Moskovcak wrote:
On 03/12/2010 06:04 PM, Denys Vlasenko wrote:
On Fri, 2010-03-12 at 17:52 +0100, Jiri Moskovcak wrote:
checks the mtime of /var/log/messages and don't read it if it didn't change since last scan.
The mtime is logically attached to a specific file. Using a static variable for it would make it impossible to figure out to which file this mtime belongs.
I suggest adding a "time_t *mtime_p" parameter to scan_syslog_file() which points to a place where old mtime may be found, and new one will be stored on return.
The parameter can be NULL, which means "do not perform scan avoidance" - for example, dumpoops will pass NULL.
Makes sense, so I changed the patch and here it is, please review.
Looks good to me. I also compile-tested it - build was ok.
crash-catcher@lists.fedorahosted.org