VOTE: List loop macros in ntoskrnl?

[Thomas Weidenmueller]

As according to our voting rules, any permanent developer may vote on
this for one week as of now.

Should the flow control macros for looping double linked lists be
allowed in ntoskrnl?

[ ] Yes
[ ] No

More on voting can be found at: http://www.reactos.com/wiki/index.php/Voting

[Gunnar Dalsnes]

Thomas Weidenmueller wrote:
> As according to our voting rules, any permanent developer may vote on
> this for one week as of now.
>
> Should the flow control macros for looping double linked lists be
> allowed in ntoskrnl?

Uhm, i have used those macros in fs\filelock.c and fs\notify.c for ages 
without anyone knowing/caring, and i "own" that code;-P I think ntoskrnl 
is too broad...

G.
> [ ] Yes
> [ ] No
>
> More on voting can be found at: http://www.reactos.com/wiki/index.php/Voting
> _______________________________________________
> Ros-dev mailing list
> [e-mail]
> http://reactos.com:8080/mailman/listinfo/ros-dev

[Magnus Olsen]

I vote no
for it make the code harder to read and undertsand a bad example why not use
maroc inntoskrnl see
file win32k/eng/gardine.c  go to middle of the file. and try figout what the
hell going on there.
that is one exmple why not use marco. I are ageinst marco when it been using
to hide
complex code that are bit ugly when it using goto.  I am using goto some
time in my code
as last resures. And hide complex code that contain goto is even worse
in my eys and try understand what the hell is going on alot harders to
figout when u are using marco
with complex code.

----- Original Message ----- 
From: "Thomas Weidenmueller" [e-mail]
To: "ReactOS Development List" [e-mail]
Sent: den 29 September 2005 12:42
Subject: [ros-dev] VOTE: List loop macros in ntoskrnl?
> As according to our voting rules, any permanent developer may vote on
> this for one week as of now.
>
> Should the flow control macros for looping double linked lists be
> allowed in ntoskrnl?
>
> [ ] Yes
> [ ] No
>
> More on voting can be found at:

http://www.reactos.com/wiki/index.php/Voting
> Ros-dev mailing list
> [e-mail]
> http://reactos.com:8080/mailman/listinfo/ros-dev
>
> -- 
> No virus found in this incoming message.
> Checked by AVG Anti-Virus.
> Version: 7.0.344 / Virus Database: 267.11.8/114 - Release Date: 2005-09-28

[Gunnar Dalsnes]

Magnus Olsen wrote:
> I vote no
> for it make the code harder to read and undertsand a bad example why not use
> maroc inntoskrnl see
> file win32k/eng/gardine.c  go to middle of the file. and try figout what the
> hell going on there.

But this is about the list macros. Are they a bad example of macro usage 
too?

But what do you suggest as an improvement to the file you pointed out? 
Would you understand it if the macros were inlined functions? I hope 
your dont mean that the code should be repeated instead of putting the 
code in a function.
> that is one exmple why not use marco. I are ageinst marco when it been using
> to hide
> complex code that are bit ugly when it using goto.  I am using goto some
> time in my code
> as last resures. And hide complex code that contain goto is even worse
> in my eys and try understand what the hell is going on alot harders to
> figout when u are using marco
> with complex code.

The list macros doesnt use goto. The list macros hide a for() loop (as 
you can tell by the "for" in its name: LIST_FOR_EACH).

There are good/bad examples of any code, macros or not. Just because 
macros can (easily?) be abused and have been abused doesnt mean we 
should never use them. To me it seems like macros is used as a 
punchingball when someone stumble upon code that is hard to understand. 
I probably dont understand more than 5% of the code in Reactos, but i 
dont blame macros for it. Used with care, macros are powerfull tools.

G.

[Thomas Weidenmueller]

Gunnar Dalsnes wrote:
> Magnus Olsen wrote:
> > I vote no
> > for it make the code harder to read and undertsand a bad example why
> > not use
> > maroc inntoskrnl see
> > file win32k/eng/gardine.c  go to middle of the file. and try figout
> > what the
> > hell going on there.
>
> But this is about the list macros. Are they a bad example of macro usage
> too?
>
> But what do you suggest as an improvement to the file you pointed out?
> Would you understand it if the macros were inlined functions? I hope
> your dont mean that the code should be repeated instead of putting the
> code in a function.
> > that is one exmple why not use marco. I are ageinst marco when it been
> > using
> > to hide
> > complex code that are bit ugly when it using goto.  I am using goto some
> > time in my code
> > as last resures. And hide complex code that contain goto is even worse
> > in my eys and try understand what the hell is going on alot harders to
> > figout when u are using marco
> > with complex code.
>
> The list macros doesnt use goto. The list macros hide a for() loop (as
> you can tell by the "for" in its name: LIST_FOR_EACH).
>
> There are good/bad examples of any code, macros or not. Just because
> macros can (easily?) be abused and have been abused doesnt mean we
> should never use them. To me it seems like macros is used as a
> punchingball when someone stumble upon code that is hard to understand.
> I probably dont understand more than 5% of the code in Reactos, but i
> dont blame macros for it. Used with care, macros are powerfull tools.
>
> G.
> _______________________________________________
> Ros-dev mailing list
> [e-mail]
> http://reactos.com:8080/mailman/listinfo/ros-dev

This thread wasn't intended for discussion. Either vote yes or no.
Please use the existing threads for discussions.

- Thomas

[KJKHyperion]

Thomas Weidenmueller wrote:

Vote (normative):
> [*] Yes
> [ ] No

Rationale (informative): it's code that would have to be written in any 
case (there's no other algorithm to traverse a list -> no added bloat), 
it's the same code over and over again (easy to introduce errors by 
retyping or copy-pasting), and it's code that cannot be implemented in 
an inline function due to deficiencies of the C language. Once the macro 
is proven correct, and possibly invariant ASSERTs are added, there will 
be no need to ever debug it

In my opinion an overblown minor issue and a sterile debate with fear 
tactics (appeal to authority, slippery slope, ad hominem) used in place 
of logic in making arguments

Additional proposal: invariant ASSERTs in list manipulation routines, 
especially the precondition that inputs are valid LIST_ENTRYs. Saved my 
ass so many times

[Gunnar Dalsnes]

KJKHyperion wrote:
> Additional proposal: invariant ASSERTs in list manipulation routines,

What does invariant mean?
> especially the precondition that inputs are valid LIST_ENTRYs. Saved my 
> ass so many times

Yeah. I miss those. I think we had something like that before(?). If 
noone objects lets readd list asserts (and no, not the "insane" variant 
that walks and validated the entire list, allthou that is very handy 
sometimes too;-)

G.

[KJKHyperion]

Gunnar Dalsnes wrote:
> > Additional proposal: invariant ASSERTs in list manipulation routines,
>
> What does invariant mean?

invariant = something that doesn't vary. All algorithms have invariants 
of some sort, and they are usually assumed (i.e. checking the invariants 
isn't part of the algorithm, and checks, if expressed in code, are 
usually only compiled in debug builds - see ASSERT). The invariant of 
all helper functions manipulating LIST_ENTRYs is that the LIST_ENTRYs 
are valid, i.e.:

ASSERT(ListEntry && ListEntry->Flink && ListEntry->Blink && 
ListEntry->Flink->Blink == ListEntry && ListEntry->Blink->Flink == 
ListEntry)

You need to ensure this is always true: before manipulating the object 
and (especially!) after you're done with it (i.e. it should be the first 
and last line of every LIST_ENTRY helper function)
> > especially the precondition that inputs are valid LIST_ENTRYs. Saved 
> > my ass so many times
>
> Yeah. I miss those. I think we had something like that before(?). If 
> noone objects lets readd list asserts (and no, not the "insane" 
> variant that walks and validated the entire list, allthou that is very 
> handy sometimes too;-)

the "insane" checks can be performed once in N times, if they are too 
much of a performance hit. And please don't hardcode calls to 
KeBugCheckEx in ASSERT - RtlAssert is where you're supposed to implement 
assertion failure logic, the ASSERT macro is just candy coating on it

[Casper Hornstrup]

Why limit it to ntoskrnl? The arguments against it applies
to source code in general, not just ntoskrnl source code.
The vote should not be limited to a particular component IMHO.
External/shared code being the exception of course.

Casper
> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]] On Behalf Of Thomas
> Weidenmueller
> Sent: 29. september 2005 12:43
> To: ReactOS Development List
> Subject: [ros-dev] VOTE: List loop macros in ntoskrnl?
>
> As according to our voting rules, any permanent developer may vote on
> this for one week as of now.
>
> Should the flow control macros for looping double linked lists be
> allowed in ntoskrnl?
>
> [ ] Yes
> [ ] No
>
> More on voting can be found at: http://www.reactos.com/wiki/index.php/Voting
> _______________________________________________
> Ros-dev mailing list
> [e-mail]
> http://reactos.com:8080/mailman/listinfo/ros-dev

[Alex Ionescu]

Thomas Weidenmueller wrote:
> As according to our voting rules, any permanent developer may vote on
> this for one week as of now.
>
> Should the flow control macros for looping double linked lists be
> allowed in ntoskrnl?
>
> [ ] Yes
> [x] No

[Richard Campbell]

Should the flow control macros for looping double linked lists be 
allowed in ntoskrnl?
[ ] Yes
[x] No

[Ge van Geldorp]

> From: Thomas Weidenmueller
>
> Should the flow control macros for looping double linked 
> lists be allowed in ntoskrnl?

[ ] Yes
[X] No

Gé van Geldorp

[Thomas Weidenmueller]

Here are the results of the vote, although two days late:
> Should the flow control macros for looping double linked lists be
> allowed in ntoskrnl?

Yes: 1
No:  4

- Thomas