Unless you are vicmarcal, my e-mail was not for you. Best regards, Alex Ionescu
On Mon, Sep 5, 2011 at 5:50 PM, Aleksey Bragin <[email protected]> wrote: > Perfect, I have a straight answer from you right away, so my trick > worked. > > Now, seriously, is that hack still needed? I understand it was needed > during early time when you developed this branch, but what about now? With > all the changes it had over time when you last worked on it? > > *From:* Alex Ionescu <[email protected]> > *Sent:* Monday, September 05, 2011 8:25 PM > *To:* ReactOS Development List <[email protected]> > *Subject:* Re: [ros-dev] [ros-diffs] [fireball] 53596: [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. > > 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]> 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 >>> 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 >>> >>> ============================================================================== >>> --- 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 > > > _______________________________________________ > 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
