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?
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
<vicmarcal@hotmail.com>
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
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,
<fireball@svn.reactos.org> 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
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev
_______________________________________________
Ros-dev mailing
list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev