[ros-dev] [ros-diffs] [hbelusca] 57400: [NTOSKRNL] Coverity code defects fixes : - Cache: CID 701441 - Config: CIDs 716570, 716669, 716760 - Dbgk: Kdbg: CIDs 716571, 515128/9, 500432 - Ex: CIDs 500156/7, 515122, 716200/...

[Pierre Schweitzer]

Then, you'd rather mark it as false-positive in Coverity interface
rather than adding redundant and useless check.

Or, if Coverity supports it, annotate code.

The issue here is Coverity, not the code.

On Fri, 2012-09-28 at 11:34 +0200, Hermès BÉLUSCA - MAÏTO wrote:
> Hi !
> 
> It's an attempt to say to Coverity that the NotifyChange variable will NEVER be NULL after the allocation (as explained in the comment), because of the fact that the code uses POOL_RAISE_IF_ALLOCATION_FAILURE. Indeed, Coverity expected a "check against NULL and fail if it is" before the usage of the NotifyChange variable and therefore warned about its usage without checking against NULL, because it's dereferenced just after the allocation call. Coverity couldn't know what the usage of the POOL_RAISE_IF_ALLOCATION_FAILURE is for. In conclusion, the aim of the assert is to remove the Coverity warning without adding code-behaviour modification.
> 
> Hermès.
> 
> 
> -----Message d'origine-----
> De : ros-dev-bounces at reactos.org [mailto:ros-dev-bounces at reactos.org] De la part de Pierre Schweitzer
> Envoyé : vendredi 28 septembre 2012 09:57
> À : ros-dev at reactos.org
> Objet : Re: [ros-dev] [ros-diffs] [hbelusca] 57400: [NTOSKRNL] Coverity code defects fixes : - Cache: CID 701441 - Config: CIDs 716570, 716669, 716760 - Dbgk: Kdbg: CIDs 716571, 515128/9, 500432 - Ex: CIDs 500156/7, 515122, 716200/...
> 
> What's the purpose of such ASSERT?
> 
> > Modified: trunk/reactos/ntoskrnl/fsrtl/notify.c
> > URL:
> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/fsrtl/notify.c?rev=57400&r1=57399&r2=57400&view=diff
> >
> ==============================================================================
> > --- trunk/reactos/ntoskrnl/fsrtl/notify.c [iso-8859-1] (original)
> > +++ trunk/reactos/ntoskrnl/fsrtl/notify.c [iso-8859-1] Thu Sep 27
> 17:16:31 2012
> > @@ -586,6 +586,13 @@
> > /* Allocate new notification */
> > NotifyChange = ExAllocatePoolWithTag(PagedPool |
> POOL_RAISE_IF_ALLOCATION_FAILURE,
> > sizeof(NOTIFY_CHANGE), 'FSrN');
> > +
> > + /*
> > + * If NotifyChange == NULL then an
> > + * exception was already raised.
> > + */
> > + ASSERT(NotifyChange != NULL);
> > +
> > RtlZeroMemory(NotifyChange, sizeof(NOTIFY_CHANGE));
> > 
> > /* Set basic information */
> > 
> 

-- 
Pierre Schweitzer <pierre at reactos.org>
System Administrator
ReactOS Foundation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 4016 bytes
Desc: not available
URL: <http://www.reactos.org/pipermail/ros-dev/attachments/20120928/e8be54db/attachment-0001.bin>