As a "non-dev" you seem to have understood the issue more than the project developer leader! Good job.
Indeed, the original code lacks a comment stating that the first assignment is how it's "supposed" to be done, but that the second assignment is currently a hack to make ReactOS work (in the original commit I made, the first line was commented out -- but a comment never added as to why). So that's my fault. You're also right that this "unneccessary assignment" is actually quite necessary, and that by removing the SECOND ChildList assignment, instead of the first, the entire functionality of the function is modified -- if anything, the first assignment should've been removed (since no comment was there explaining why it was needed). So yes, good job on understanding both these issues, and for none of the devs seemingly able to see your correct logic... Best regards, Alex Ionescu On Mon, Sep 5, 2011 at 1:03 PM, Vicmarcal <[email protected]> wrote: > Nope,I have asked in #reactos-dev too about that removal just after the > commiting was done. > To begin I dont understand the logic in the original code about reassigning > the value to the ChildList two lines later(unless the first assignation is > not needed at all) > And after the commit I dont understand the commit message "unnecesary > assignments".Now,after removing the second assignment,the behavior should be > totally different(unless &Kcb->ValueCache; is == &KeyNode->ValueList; , > which I doubt). So if the removal is correct,then it wasnt an "unnecesary > change" but a "fixing a bug". > Anyway I am not a dev, so I express my doubts here too if anyone can light > me a little :) > > Enviado desde mi iPhone > > El 05/09/2011, a las 13:48, "Alex Ionescu" <[email protected]> escribió: > > Uhhhh... > > Am I really the *only* one who sees a problem here? > > ChildList = &Kcb->ValueCache; > - ChildList = (PCACHED_CHILD_LIST)&KeyNode->ValueList; > > Best regards, > Alex Ionescu > > > On Mon, Sep 5, 2011 at 10:54 AM, < <[email protected]> > [email protected]> wrote: > >> Author: fireball >> Date: Mon Sep 5 09:54:20 2011 >> New Revision: 53596 >> >> URL: <http://svn.reactos.org/svn/reactos?rev=53596&view=rev> >> http://svn.reactos.org/svn/reactos?rev=53596&view=rev >> Log: >> [NTOS/CONFIG] >> - Remove unnecessary assignments. Spotted by PVS and Dmitry Chapyshev. >> This may change the behaviour of that codepath, so test results are going to >> be observed. >> >> Modified: >> trunk/reactos/ntoskrnl/config/cmvalche.c >> >> Modified: trunk/reactos/ntoskrnl/config/cmvalche.c >> URL: >> <http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/config/cmvalche.c?rev=53596&r1=53595&r2=53596&view=diff> >> http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/config/cmvalche.c?rev=53596&r1=53595&r2=53596&view=diff >> >> ============================================================================== >> --- trunk/reactos/ntoskrnl/config/cmvalche.c [iso-8859-1] (original) >> +++ trunk/reactos/ntoskrnl/config/cmvalche.c [iso-8859-1] Mon Sep 5 >> 09:54:20 2011 >> @@ -49,7 +49,6 @@ >> PHHIVE Hive; >> PCACHED_CHILD_LIST ChildList; >> HCELL_INDEX CellToRelease; >> - PCM_KEY_NODE KeyNode; >> >> /* Set defaults */ >> *ValueListToRelease = HCELL_NIL; >> @@ -58,8 +57,6 @@ >> /* Get the hive and value cache */ >> Hive = Kcb->KeyHive; >> ChildList = &Kcb->ValueCache; >> - KeyNode = (PCM_KEY_NODE)HvGetCell(Hive, Kcb->KeyCell); >> - ChildList = (PCACHED_CHILD_LIST)&KeyNode->ValueList; >> >> /* Check if the value is cached */ >> if (CmpIsValueCached(ChildList->ValueList)) >> @@ -212,7 +209,6 @@ >> BOOLEAN IndexIsCached; >> ULONG i = 0; >> HCELL_INDEX Cell = HCELL_NIL; >> - PCM_KEY_NODE KeyNode; >> >> /* Set defaults */ >> *CellToRelease = HCELL_NIL; >> @@ -221,8 +217,6 @@ >> /* Get the hive and child list */ >> Hive = Kcb->KeyHive; >> ChildList = &Kcb->ValueCache; >> - KeyNode = (PCM_KEY_NODE)HvGetCell(Hive, Kcb->KeyCell); >> - ChildList = (PCACHED_CHILD_LIST)&KeyNode->ValueList; >> >> /* Check if the child list has any entries */ >> if (ChildList->Count != 0) >> >> >> > _______________________________________________ > Ros-dev mailing list > [email protected] > http://www.reactos.org/mailman/listinfo/ros-dev > > > _______________________________________________ > Ros-dev mailing list > [email protected] > http://www.reactos.org/mailman/listinfo/ros-dev > >
_______________________________________________ Ros-dev mailing list [email protected] http://www.reactos.org/mailman/listinfo/ros-dev
