Re: [ros-dev] [EVENTVWR/EVENTLOG] Fix a double off-by-one bug

[Love Nystrom]

Hi Gabriel, Eric,

Isn't the real issue why LogFile->Header.CurrentRecordNumber is off by one ?
If that is found and fixed, this patch will make the error re-emerge as an
off by -1.
And if CurrentRecordNumber is actually used as the next *pending* record
number,
the identifier is somewhat mis-leading .. ?

Just my penny to the pot
Best Regards
L.

---------- Forwarded message ----------
>   From: Gabriel ilardi <gabrielilardi(a)hotmail.it>
>  To: ros-dev <ros-dev(a)reactos.org>
>  Date: Mon, 2 May 2011 21:50:03 +0200
>  Subject: Re: [ros-dev] [ros-diffs] [ekohl] 51558: [EVENTVWR/EVENTLOG] Fix a
>  double off-by-one bug: - The eventlog service was reporting one event more
>  than was available (+1). - The event viewer did not display the latest event
>  from...
>   Great work on event service and event viewer... keep it up Eric :)
>
>
> >   Date: Mon, 2 May 2011 19:38:24 +0000
> >  To: ros-diffs(a)reactos.org
> >  From: ekohl(a)svn.reactos.org
> >  Subject: [ros-diffs] [ekohl] 51558: [EVENTVWR/EVENTLOG] Fix a double 
>  off-by-one
> bug: - The eventlog service was reporting one event more than was
>  available (+1). - The event viewer did not display the latest event from...
> >  
> >  Author: ekohl
> >  Date: Mon May 2 19:38:23 2011
> >  New Revision: 51558
> >
> >  URL: http://svn.reactos.org/svn/reactos?rev=51558&view=rev
> >  Log:
> >  [EVENTVWR/EVENTLOG]
> >  Fix a double off-by-one bug:
> >  - The eventlog service was reporting one event more than was available 
>  (+1).
> >   - The event viewer did not display the latest
> > event from the eventlog 
>  service (-1).
> >  
> >  See issue #6182 for more details.
> >
> >  Modified:
> >  trunk/reactos/base/applications/mscutils/eventvwr/eventvwr.c
> >  trunk/reactos/base/services/eventlog/rpc.c
> >
> >  Modified: trunk/reactos/base/applications/mscutils/eventvwr/eventvwr.c
> >  URL: 
> http://svn.reactos.org/svn/reactos/trunk/reactos/base/applications/mscutils…
> >   
> ==============================================================================
> >   ---
> > trunk/reactos/base/applications/mscutils/eventvwr/eventvwr.c 
>  [iso-8859-1]
> (original)
> >   +++
> > trunk/reactos/base/applications/mscutils/eventvwr/eventvwr.c 
>  [iso-8859-1] Mon May 2
> 19:38:23 2011
> >   @@ -503,7 +503,7 @@
> >  HWND hwndDlg;
> >  HANDLE hEventLog;
> >  EVENTLOGRECORD *pevlr;
> >  - DWORD dwRead, dwNeeded, dwThisRecord, dwTotalRecords = 0, 
>  dwCurrentRecord = 1,
> dwRecordsToRead = 0, dwFlags, dwMaxLength;
> >   + DWORD dwRead, dwNeeded, dwThisRecord,
> > dwTotalRecords = 0, 
>  dwCurrentRecord = 0, dwRecordsToRead = 0, dwFlags,
> dwMaxLength;
>  > LPWSTR lpSourceName;
>  > LPWSTR lpComputerName;
>  > LPSTR lpData;
> >   
>  > Modified:
> trunk/reactos/base/services/eventlog/rpc.c
>  > URL:
>
> http://svn.reactos.org/svn/reactos/trunk/reactos/base/services/eventlog/rpc…
> >   
> ==============================================================================
> >   --- trunk/reactos/base/services/eventlog/rpc.c
> > [iso-8859-1] (original)
> >  +++ trunk/reactos/base/services/eventlog/rpc.c [iso-8859-1] Mon May 2 
>  19:38:23
> 2011
>  > @@ -199,6 +199,7 @@
>  > DWORD *NumberOfRecords)
>  > {
>  > PLOGHANDLE lpLogHandle;
>  > + DWORD dwRecords;
> >   
>  > lpLogHandle =
> ElfGetLogHandleEntryByHandle(LogHandle);
>  > if (!lpLogHandle)
>  > @@ -206,7 +207,9 @@
>  > return STATUS_INVALID_HANDLE;
>  > }
> >   
>  > - *NumberOfRecords =
> lpLogHandle->LogFile->Header.CurrentRecordNumber;
>  > + dwRecords = lpLogHandle->LogFile->Header.CurrentRecordNumber;
>  > +
>  > + *NumberOfRecords = (dwRecords > 0) ? (dwRecords - 1) : 0;
> >   
>  > return STATUS_SUCCESS;
>  > }
> >   
> >  
>

[Eric Kohl]

Hi Love,

I just committed a fix for the calculation of the available number of 
event records. It addresses the issues you mentioned.

And this is how it works:
When a new log file is created, the first (or oldest) record gets the 
record number 1. With each new event record the record number is 
incremented by 1. To get the number of records in the log file we need 
to subtract the OldestRecordNumber from the CurrentRecordNumber. If the 
log file is empty the OldestRecordNumber will be 0. In this case we will 
return 0 for the number of event records.

Now we call clear a filled log file and fill it with new records. The 
oldest record will not be record number 1 but we can still return the 
true number of records in the log file.


Regards
Eric

>   
>  Hi Gabriel, Eric,
>  
>  Isn't the real issue why LogFile->Header.CurrentRecordNumber is off by one ?
>  If that is found and fixed, this patch will make the error re-emerge as 
>  an off by -1.
>  And if CurrentRecordNumber is actually used as the next *pending* record 
>  number,
>  the identifier is somewhat mis-leading .. ?
>  
>  Just my penny to the pot
>  Best Regards
>  L.
>  
>      ---------- Forwarded message ----------
>      From: Gabriel ilardi <gabrielilardi(a)hotmail.it
>      <mailto:[e-mail]>>
>      To: ros-dev &lt;ros-dev(a)reactos.org <mailto:[e-mail]>>
>      Date: Mon, 2 May 2011 21:50:03 +0200
>      Subject: Re: [ros-dev] [ros-diffs] [ekohl] 51558:
>      [EVENTVWR/EVENTLOG] Fix a double off-by-one bug: - The eventlog
>      service was reporting one event more than was available (+1). - The
>      event viewer did not display the latest event from...
>      Great work on event service and event viewer... keep it up Eric :)
>  
>  
> >   Date: Mon, 2 May 2011 19:38:24 +0000
> >  To: ros-diffs(a)reactos.org <mailto:[e-mail]>
> >  From: ekohl(a)svn.reactos.org <mailto:[e-mail]>
> >  Subject: [ros-diffs] [ekohl] 51558: [EVENTVWR/EVENTLOG] Fix a 
>      double
> off-by-one bug: - The eventlog service was reporting one
>      event more than was available (+1). - The event viewer did not
>      display the latest event from...
> >  
> >  Author: ekohl
> >  Date: Mon May 2 19:38:23 2011
> >  New Revision: 51558
> >
> >  URL: http://svn.reactos.org/svn/reactos?rev=51558&view=rev 
>     
> <http://svn.reactos.org/svn/reactos?rev=51558&view=rev>
> >   Log:
> >  [EVENTVWR/EVENTLOG]
> >  Fix a double off-by-one bug:
> >  - The eventlog service was reporting one event more than was 
>      available (+1).
> >   - The event viewer did not display the latest
> > event from the 
>      eventlog service (-1).
> >  
> >  See issue #6182 for more details.
> >
> >  Modified:
> >  trunk/reactos/base/applications/mscutils/eventvwr/eventvwr.c
> >  trunk/reactos/base/services/eventlog/rpc.c
> >
> >  Modified: 
>      trunk/reactos/base/applications/mscutils/eventvwr/eventvwr.c
> >   URL: 
>     
> http://svn.reactos.org/svn/reactos/trunk/reactos/base/applications/mscutils…
>     
> <http://svn.reactos.org/svn/reactos/trunk/reactos/base/applications/mscutils/eventvwr/eventvwr.c?rev=51558&r1=51557&r2=51558&view=diff>
> >   
>     
> ==============================================================================
> >   ---
> > trunk/reactos/base/applications/mscutils/eventvwr/eventvwr.c 
>      [iso-8859-1]
> (original)
> >   +++
> > trunk/reactos/base/applications/mscutils/eventvwr/eventvwr.c 
>      [iso-8859-1] Mon
> May 2 19:38:23 2011
> >   @@ -503,7 +503,7 @@
> >  HWND hwndDlg;
> >  HANDLE hEventLog;
> >  EVENTLOGRECORD *pevlr;
> >  - DWORD dwRead, dwNeeded, dwThisRecord, dwTotalRecords = 0, 
>      dwCurrentRecord =
> 1, dwRecordsToRead = 0, dwFlags, dwMaxLength;
> >   + DWORD dwRead, dwNeeded, dwThisRecord,
> > dwTotalRecords = 0, 
>      dwCurrentRecord = 0, dwRecordsToRead = 0, dwFlags,
> dwMaxLength;
>       > LPWSTR lpSourceName;
>       > LPWSTR lpComputerName;
>       > LPSTR lpData;
> >   
>       > Modified:
> trunk/reactos/base/services/eventlog/rpc.c
> >   URL: 
>     
> http://svn.reactos.org/svn/reactos/trunk/reactos/base/services/eventlog/rpc…
>     
> <http://svn.reactos.org/svn/reactos/trunk/reactos/base/services/eventlog/rpc.c?rev=51558&r1=51557&r2=51558&view=diff>
> >   
>     
> ==============================================================================
> >   --- trunk/reactos/base/services/eventlog/rpc.c
> > [iso-8859-1] 
>      (original)
> >   +++ trunk/reactos/base/services/eventlog/rpc.c
> > [iso-8859-1] Mon 
>      May 2 19:38:23 2011
>       > @@ -199,6 +199,7 @@
>       > DWORD *NumberOfRecords)
>       > {
>       > PLOGHANDLE lpLogHandle;
>       > + DWORD dwRecords;
> >   
>       > lpLogHandle =
> ElfGetLogHandleEntryByHandle(LogHandle);
>       > if (!lpLogHandle)
>       > @@ -206,7 +207,9 @@
>       > return STATUS_INVALID_HANDLE;
>       > }
> >   
>       > - *NumberOfRecords =
>      lpLogHandle->LogFile->Header.CurrentRecordNumber;
>       > + dwRecords = lpLogHandle->LogFile->Header.CurrentRecordNumber;
>       > +
>       > + *NumberOfRecords = (dwRecords > 0) ? (dwRecords - 1) : 0;
> >   
>       > return STATUS_SUCCESS;
>       > }
> >   
> >  
>  
>  
>  
>  ------------------------------------------------------------------------
>  
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev