Re: [ros-svn] [gdalsnes] 18113: -reorder InsertXscendingOrder macro argument order and update uses

[Alex Ionescu]

gdalsnes(a)svn.reactos.com wrote:

>  -reorder InsertXscendingOrder macro argument order and
> update uses
> -start to macrofy list enums in ntoskrnl using LIST_FOR_EACH macros
> -use IsListEmpty some places instead of manual coding
> -profile.c:KeStartProfile() zero correct buffer in (and not the NULL ptr;-)
> -improve LIST_FOR_EACH macros so that the iterator is set to NULL at end of enum
> -kthread.c:KiServiceCheck() set the shadowtable before calling the win32k proc/thread init
> funcs
> -apc.c:KiInsertQueueApc() insert special apcs in correct fifo order 
> -apc.c:KeFlushQueueApc() simplify 
> -timer.c:KiExpireTimers() calling RemoveEntryList on a possibly empty list is not ok
>
>
> Updated files:
> trunk/reactos/include/reactos/helper.h
> trunk/reactos/ntoskrnl/io/device.c
> trunk/reactos/ntoskrnl/io/driver.c
> trunk/reactos/ntoskrnl/io/file.c
> trunk/reactos/ntoskrnl/io/fs.c
> trunk/reactos/ntoskrnl/io/irp.c
> trunk/reactos/ntoskrnl/io/pnpnotify.c
> trunk/reactos/ntoskrnl/io/pnproot.c
> trunk/reactos/ntoskrnl/ke/apc.c
> trunk/reactos/ntoskrnl/ke/bug.c
> trunk/reactos/ntoskrnl/ke/kqueue.c
> trunk/reactos/ntoskrnl/ke/kthread.c
> trunk/reactos/ntoskrnl/ke/profile.c
> trunk/reactos/ntoskrnl/ke/queue.c
> trunk/reactos/ntoskrnl/ke/timer.c
> trunk/reactos/subsys/system/usetup/partlist.c
> trunk/reactos/subsys/win32k/ntuser/class.c
>
> __________________________
>  
Hi,

I'm going to have to voice my extreme outcry against this patch. It is 
replacing clear and easily debuggable code by non-standard macros which 
are impossible to debug. It is also going to create problems when people 
which are not used to them (such as myself):

1) Have no idea what some LIST_FOR_EACH_SAFE thing is doing vs 
LIST_FOR_EACH vs InsertListLifoSafeMaybeLoopOrWhoKnowsWhat
2) Will never code using these macros, and thus the source will become 
full of both methods, which is totally ugly.

Sometimes, especially in kernel code, it's much better to have an 
expanded logic then to use a macro. I wrote a great majority of the code 
that has just been, imho, pollutted with these macros, and I don't 
appreciate for someone to barge in and change my code for some macro 
that I don't understand, nor wish to, because it only causes problems. I 
take great care of the code I write and "own" it (in the programming 
term) so that if a bug in it arises, I can quickly identify it and be 
held responsible for it. The addition of these macros hinders that effort.

I don't mind someone using them inside his or her own code. I just mind 
their usage in ntoskrnl, and even more so when it overwrites the clean, 
clear code that I've written and am used to debugging.

Best regards,
Alex Ionescu

[Thomas Weidenmueller]

Alex Ionescu wrote:
>   Hi,
>  
>  I'm going to have to voice my extreme outcry against this patch. It is
>  replacing clear and easily debuggable code by non-standard macros which
>  are impossible to debug. It is also going to create problems when people
>  which are not used to them (such as myself):
>  
>  1) Have no idea what some LIST_FOR_EACH_SAFE thing is doing vs
>  LIST_FOR_EACH vs InsertListLifoSafeMaybeLoopOrWhoKnowsWhat
>  2) Will never code using these macros, and thus the source will become
>  full of both methods, which is totally ugly.
>  
>  Sometimes, especially in kernel code, it's much better to have an
>  expanded logic then to use a macro. I wrote a great majority of the code
>  that has just been, imho, pollutted with these macros, and I don't
>  appreciate for someone to barge in and change my code for some macro
>  that I don't understand, nor wish to, because it only causes problems. I
>  take great care of the code I write and "own" it (in the programming
>  term) so that if a bug in it arises, I can quickly identify it and be
>  held responsible for it. The addition of these macros hinders that effort.
>  
>  I don't mind someone using them inside his or her own code. I just mind
>  their usage in ntoskrnl, and even more so when it overwrites the clean,
>  clear code that I've written and am used to debugging.
>  
>  Best regards,
>  Alex Ionescu 
I definitely agree.

- Thomas

[Steven Edwards]

Hi,

--- Alex Ionescu <ionucu(a)videotron.ca> wrote:
>   I don't mind someone using them inside his or her
> own code. I just mind 
>  their usage in ntoskrnl, and even more so when it overwrites the clean, 
>  clear code that I've written and am used to debugging. 
I think this change should be reverted as well. If Alex is maintaining this code and he
does not
want to use these macros I don't think we should force them on anyone. While it would
be nice to
use the Wine list macros in a standard way I just can't agree with it if the people
that are
hacking on the kernel the most have a problem with it.

Thanks
Steven


__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com

[Gunnar Dalsnes]

If Alex doesnt like it ill revert it in the files that he "own", but 
this "i own the code" stuff has gone too far.

Any person can see that those macros produce much cleaner code, and 
without any measureable performance loss. And the stuff with "macros 
screw my debugging", all list manipulation functions are already macros.

Steven Edwards wrote:
>   Hi,
>  
>  --- Alex Ionescu <ionucu(a)videotron.ca> wrote:
>  
> >  I don't mind someone using them inside his or
> > her own code. I just mind 
> > their usage in ntoskrnl, and even more so when it overwrites the clean, 
> > clear code that I've written and am used to debugging. 
>  
>  
>  I think this change should be reverted as well. If Alex is maintaining this code and he
> does not
>  want to use these macros I don't think we should force them on anyone. While it would
> be nice to
>  use the Wine list macros in a standard way I just can't agree with it if the people
> that are
>  hacking on the kernel the most have a problem with it.
>  
>  Thanks
>  Steven
>  
>  
>  __________________________________________________
>  Do You Yahoo!?
>  Tired of spam?  Yahoo! Mail has the best spam protection around 
>  http://mail.yahoo.com 
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.com
>  http://reactos.com:8080/mailman/listinfo/ros-dev
>  
>  
>

[Thomas Weidenmueller]

Gunnar Dalsnes wrote:
>   If Alex doesnt like it ill revert it in the files that
> he "own", but
>  this "i own the code" stuff has gone too far.
>  
>  Any person can see that those macros produce much cleaner code, and
>  without any measureable performance loss. And the stuff with "macros
>  screw my debugging", all list manipulation functions are already macros. 
I don't think it's about "owning" code - I also strongly dislike
LIST_FOR_EACH and especially LIST_FOR_EACH_SAFE. I also prefer the loops
as they have been implemented before. Sometimes also such minor
optimizations help a lot. And if one doesn't know how to deal with
double linked lists, one shouldn't use them, IMO.

- Thomas

[Gunnar Dalsnes]

>   I also prefer the loops
>  as they have been implemented before. Sometimes also such minor
>  optimizations help a lot.  
Walking the list by handcoding is not an optimization. Those macros are 
just as fast as doing it manually.

>   And if one doesn't know how to deal with
>  double linked lists, one shouldn't use them, IMO. 
Cleaner code (less code) is the point here, not knowing how to deal with 
lists. Less code = better code.

>   
>  - Thomas
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.com
>  http://reactos.com:8080/mailman/listinfo/ros-dev
>  
>  
>

[Thomas Weidenmueller]

Gunnar Dalsnes wrote:
>   Walking the list by handcoding is not an optimization.
> Those macros are
>  just as fast as doing it manually. 
Yes it is, not always but especially the kernel
sometimes have time
critical operations where any unnecessary overhead could have an impact
on performance.

>   Cleaner code (less code) is the point here, not
> knowing how to deal with
>  lists. Less code = better code. 
That's not always true. I remember the old
Nt/KeWaitForSingleObject
implementation.... total mess and slow as hell...

At least I'm not going to use these non-standard macros.

- Thomas

[Alex Ionescu]

Gunnar Dalsnes wrote:

>   If Alex doesnt like it ill revert it in the files that
> he "own", but 
>  this "i own the code" stuff has gone too far. 
It's not about "owning". Call it "maintaining" if you want, like
Steven 
said. And it's not "Alex" doesn't like it, it's erm, Alex, Steven, 
Thomas, Casper, Hartmut and probably others. I'll go in more detail on 
the technical reason why, at least for me, the macros are unacceptable.  
(Other then the fact that macros in the programming world are considered 
to be as bad as goto.) Here's something I can do that your macros remove:

HeadList = &Foo.Flink
DPRINT1("List head at: %p\n", HeadList);
Entry = HeadList->Flink;
DPRINT1("Entry at: %p\n", Entry);
while (Entry != HeadList)
{
       DPRINT1("Getting object\n");
       Object = CONTAINING_RECORD(Entry, OBJECT, Link);
       DPRINT1("Got the object: %p\n", Object);
      
       <.... do stuff>

       DPRINT1("About to remove entry from list\n");
       RemoveEntryList(&Object->Link);

      DPRINT1("Entry removed, moving to: %p\n", Entry->Flink);
      Entry = Entry->Flink;
}

In the current state that ReactOS + the kernel + debugging facilities 
are, that is sometimes the only way to find out what's crashing during a 
list .

>  
>  Any person can see that those macros produce much cleaner code, and 
>  without any measureable performance loss. And the stuff with "macros 
>  screw my debugging", all list manipulation functions are already macros. 
No they're not. They're inlined functions. And they do 
insertion/removal, not parsing lists. See above why parsing should be 
expanded.

Best regards,
Alex Ionescu

[Gunnar Dalsnes]

Alex Ionescu wrote:
>   Gunnar Dalsnes wrote:
>  
> >   If Alex doesnt like it ill revert it in the files
> > that he "own", but 
> >  this "i own the code" stuff has gone too far. 
>  
>  
>  It's not about "owning". Call it "maintaining" if you want, like
> Steven 
>  said. And it's not "Alex" doesn't like it, it's erm, Alex, Steven,
>
>  Thomas, Casper, Hartmut and probably others.
>  I'll go in more detail on 
>  the technical reason why, at least for me, the macros are unacceptable.  
>  (Other then the fact that macros in the programming world are considered 
>  to be as bad as goto.) Here's something I can do that your macros remove:
>  
>  HeadList = &Foo.Flink
>  DPRINT1("List head at: %p\n", HeadList);
>  Entry = HeadList->Flink;
>  DPRINT1("Entry at: %p\n", Entry);
>  while (Entry != HeadList)
>  {
>        DPRINT1("Getting object\n");
>        Object = CONTAINING_RECORD(Entry, OBJECT, Link);
>        DPRINT1("Got the object: %p\n", Object);
>             <.... do stuff>
>  
>        DPRINT1("About to remove entry from list\n");
>        RemoveEntryList(&Object->Link);
>  
>       DPRINT1("Entry removed, moving to: %p\n", Entry->Flink);
>       Entry = Entry->Flink;
>  } 
If you come across a crash during an enum you just temporarily change 
the code to hunt the bug, just like normal bug hunting. Some of the list 
enums i changed were on this form...

for (HeadList = &Foo.Flink; Entry != HeadList; Entry = Entry->Flink)
{
    Object = CONTAINING_RECORD(Entry, OBJECT, Link);
    Object->stuff
}

...and if you had to "debug" it like you showed youd had to edit the 
code just as well. But for normal not-in-process-of-being-debugged-code 
i think the above code sux (even without the dprints).

>   
>  In the current state that ReactOS + the kernel + debugging facilities 
>  are, that is sometimes the only way to find out what's crashing during a 
>  list .
>  
> >  
> >  Any person can see that those macros produce much cleaner code, and 
> >  without any measureable performance loss. And the stuff with "macros 
> >  screw my debugging", all list manipulation functions are already macros. 
>  
>  
>  No they're not. They're inlined functions. And they do 
>  insertion/removal, not parsing lists. See above why parsing should be 
>  expanded. 
Yeah, i see now they are inlined in latest DDKs...

>

[Phillip Susi]

I'd just like to chime in here with my opinion.  About a year ago I had 
the idea of introducing macros for error handling instead of explicit 
gotos.  We had some discussion on the matter, and I read several papers 
on the web, and came to agree that use of macros that change flow 
control is anathma.  It makes the code completely unreadable and 
unmaintainable.

At first it seemed like a great idea to hide the "nasty" gotos and have 
to type less while hammering out the code, but in the end, even when you 
are familliar with the macros, it looks ugly, and for someone who isn't 
familliar with the macros, it is utterly unreadable.

Recently I was reading over some apache code and they make extensive use 
of macros for flow control.  I found myself becoming frustrated to no 
end because I could not figure out what the hell was going on.  They 
looked like nice, neat function calls, but I could not for the life of 
me figure out where the code was that was being called.  It wouldn't 
have looked so deceptively simple if they coded out what was going on 
explicitly instead of using macros, but it would have been easy to 
understand, and why should something look deceptively simple when it 
really isn't?  That only leads to confusion when trying to understand 
what's going on under the hood.

[Gunnar Dalsnes]

Phillip Susi wrote:
>   I'd just like to chime in here with my opinion. 
> About a year ago I had 
>  the idea of introducing macros for error handling instead of explicit 
>  gotos.  We had some discussion on the matter, and I read several papers 
>  on the web, and came to agree that use of macros that change flow 
>  control is anathma.  It makes the code completely unreadable and 
>  unmaintainable. 
In win32k i use macros for flow control in the NtUser syscalls 
(functionally like a try/finally block) where its very important that 
some cleanup (release a lock) is always done:

BOOL NtFunc()
{
   DECLARE_RETURN(BOOL);

   Lock();
   if (Stuff) RETURN(FALSE);
   ....
   RETURN(TRUE);

CLEANUP:
   Unlock(Stuff);
   DPRINT1("NtFunc returned %i\n", _ret_);
   END_CLEANUP;
}

>   
>  At first it seemed like a great idea to hide the "nasty" gotos and have 
>  to type less while hammering out the code, but in the end, even when you 
>  are familliar with the macros, it looks ugly, and for someone who isn't 
>  familliar with the macros, it is utterly unreadable. 
If we should only invent stuff that ppl _already_ understand we just had 
to stop inventing.

>   
>  Recently I was reading over some apache code and they make extensive use 
>  of macros for flow control.  I found myself becoming frustrated to no 
>  end because I could not figure out what the hell was going on.  They 
>  looked like nice, neat function calls, but I could not for the life of 
>  me figure out where the code was that was being called. 
So if they had been function calls and not macros it would have been 
easier??? Or should we stop using functions also? ;-P

>   It wouldn't 
>  have looked so deceptively simple if they coded out what was going on 
>  explicitly instead of using macros, but it would have been easy to 
>  understand, and why should something look deceptively simple when it 
>  really isn't?   
If you cant find a macro (simple text search) you probably wouldnt have 
understod the code _without_ the macros either;-P

But off course you have to learn how the macros work to understand and 
utilize them, but that goes for any code. I cant see how learning a 
function call with 10+ params (typical nt syscall func) should be any 
easier than learning a simple macro. Thats just being lazy.

G.

[Ge van Geldorp]

>   From: Gunnar Dalsnes
>  
>  Phillip Susi wrote:
> >   I'd just like to chime in here with my
> > opinion.  About a year ago I 
> >  had the idea of introducing macros for error handling instead of 
> >  explicit gotos.  We had some discussion on the matter, and I read 
> >  several papers on the web, and came to agree that use of 
> >  macros that change flow control is anathma.  It makes the code
> >  completely unreadable and unmaintainable. 
>  
>  In win32k i use macros for flow control in the NtUser 
>  syscalls (functionally like a try/finally block) where its 
>  very important that some cleanup (release a lock) is always done:
>  
>  BOOL NtFunc()
>  {
>     DECLARE_RETURN(BOOL);
>  
>     Lock();
>     if (Stuff) RETURN(FALSE);
>     ....
>     RETURN(TRUE);
>  
>  CLEANUP:
>     Unlock(Stuff);
>     DPRINT1("NtFunc returned %i\n", _ret_);
>     END_CLEANUP;
>  } 
And I think these macro's are a perfect example of Phillip's point. I have
no idea how the flow of control is without looking at the macro definitions.
For you it's easy, you wrote them, but for the rest of us it is obfuscating.
I didn't like it when you introduced this stuff in win32k.

Gé van Geldorp.

[Gunnar Dalsnes]

Ge van Geldorp wrote:
> >  From: Gunnar
> > Dalsnes
> >
> > Phillip Susi wrote:
> >
> > >  I'd just like to chime in here with my
> > > opinion.  About a year ago I 
> > > had the idea of introducing macros for error handling instead of 
> > > explicit gotos.  We had some discussion on the matter, and I read 
> > > several papers on the web, and came to agree that use of 
> > > macros that change flow control is anathma.  It makes the code
> > > completely unreadable and unmaintainable. 
> > In win32k i use macros for flow control in the NtUser 
> > syscalls (functionally like a try/finally block) where its 
> > very important that some cleanup (release a lock) is always done:
> >
> > BOOL NtFunc()
> > {
> >    DECLARE_RETURN(BOOL);
> >
> >    Lock();
> >    if (Stuff) RETURN(FALSE);
> >    ....
> >    RETURN(TRUE);
> >
> > CLEANUP:
> >    Unlock(Stuff);
> >    DPRINT1("NtFunc returned %i\n", _ret_);
> >    END_CLEANUP;
> > } 
>  
>  
>  And I think these macro's are a perfect example of Phillip's point. I have
>  no idea how the flow of control is without looking at the macro definitions. 
Sure you do, if you try _reeeealy_ hard;-P Ok, those RETURN's set the 
_ret_ value and transfer control to the CLEANUP block. END_CLEANUP does 
the actual return with the value passed in RETURN. If you fallthru to 
the CLEANUP block you will assert so you must RETURN before the CLEANUP 
block. The CLEANUP block can be seen as an externel function but for 
obvious reasons it needs to be in the function... Hope this helps:-D

>   For you it's easy, you wrote them, but for the
> rest of us it is obfuscating.
>  I didn't like it when you introduced this stuff in win32k.
>   
The alternative is: do the cleanup at every return, use goto or use 
try/finally.
1)Cleanup at every return is madness. Most functions in ros do a large 
amount of cleanup at each return and I sometimes spot mistakes where 
one/more return misses some cleanup. Those errors are _hard_ to find.
2)Using gotos are much more ugly imo.
3)try/finally could be used if we supported returning from a try block, 
thou this is not really recomended either because of unwinding. The 
common solution is to goto to the end of the try block, but then we are 
back to start.

G.

[Ge van Geldorp]

>   From: Gunnar Dalsnes
>
> >   And I think these macro's are a perfect
> > example of 
> >  Phillip's point. I have no idea how the flow of
> >  control is without looking at the macro definitions. 
>  
>  Sure you do, if you try _reeeealy_ hard;-P 
No, really, I don't <<without looking at the macro definitions>>. RETURN
sounds much like return, it is non-obvious that they're actually goto's to
CLEANUP. Ofcourse, I figured it out when you committed that stuff 3 weeks
ago, but when looking at it last night it was again non-obvious to me.
On the other hand, I had no problem whatsoever figuring out the macro-free
code that Nathan posted:

BOOL NtFunc()
{
  BOOL bResult;
  void *pPointer = NULL;

  Lock();

  if (Stuff)
  {
    bResult = FALSE;
    goto cleanup;
  }
  ....

  bResult = TRUE;

cleanup:
  if (pPointer)
    free(pPointer);
  Unlock(stuff);
  DPRINT1("NtFunc returned %i\n", bResult);
  return bResult;
}

>   2)Using gotos are much more ugly imo. 
Oh, so goto's are acceptable if and only if you hide them out of sight?

Gé van Geldorp.

[Gunnar Dalsnes]

Ge van Geldorp wrote:
> >  From: Gunnar
> > Dalsnes
> >
> >
> > >  And I think these macro's are a perfect
> > > example of 
> > > Phillip's point. I have no idea how the flow of
> > > control is without looking at the macro definitions. 
> > Sure you do, if you try _reeeealy_ hard;-P 
>  
>  
>  No, really, I don't <<without looking at the macro definitions>>. RETURN
>  sounds much like return, it is non-obvious that they're actually goto's to
>  CLEANUP. Ofcourse, I figured it out when you committed that stuff 3 weeks
>  ago, but when looking at it last night it was again non-obvious to me.
>  On the other hand, I had no problem whatsoever figuring out the macro-free
>  code that Nathan posted: 
Yes, but how is this different from someone not knowing/understanding 
that a finally block is called when returning from a try block? I may 
very well think the the finally block is only executed if i run at the 
end of the try block. But i _learned_ and figured out how it works. And 
now i _remeber_. But its not that same you say, because the macro _can_ 
be implemented by hardcoding, while try/finally cannot. Uhm, try/finally 
in ros IS macros;-P Noone said, "kjk, s*rew you and your seh macros." 
"This belongs in the compiler." "I refuse to learn how to use those ugly 
seh macros."

>   
>  BOOL NtFunc()
>  {
>    BOOL bResult;
>    void *pPointer = NULL;
>  
>    Lock();
>  
>    if (Stuff)
>    {
>      bResult = FALSE;
>      goto cleanup;
>    }
>    ....
>  
>    bResult = TRUE;
>  
>  cleanup:
>    if (pPointer)
>      free(pPointer);
>    Unlock(stuff);
>    DPRINT1("NtFunc returned %i\n", bResult);
>    return bResult;
>  }
>  
>  
> >  2)Using gotos are much more ugly imo. 
>  
>  
>  Oh, so goto's are acceptable if and only if you hide them out of sight? 
No, i think gotos are ok internally but i dont like them for return. 
First set a retval and then goto to the end. ugh...ly. I didnt make 
those macros so i could type less. I made they so i can _read_ less. 
Thats the point. Readability. When looking at code i like to quickly 
spot the points of return. In complex code, and if it already have 
gotos, its confusing. Having a reserved word like RETURN is also nice 
for sytax highlightning (and its actually the same _word_ as normally 
used for return;-). Making up a mind about what the code does very 
quickly is nice, and with RETURN i can do that just as fast (faster) as 
with return.

>    
Im sure all of you would like those macros if you didnt refuse to learn- 
and use them. But as long as you do you will off course hate them. I 
hate all the stuff i dont understand as well.

G.

[Thomas Weidenmueller]

Gunnar Dalsnes wrote:
>   But its not that same you say, because the macro
> _can_
>  be implemented by hardcoding, while try/finally cannot. Uhm, try/finally
>  in ros IS macros;-P Noone said, "kjk, s*rew you and your seh macros."
>  "This belongs in the compiler." "I refuse to learn how to use those ugly
>  seh macros." 
That's because there's no other way to get SEH that we
really need into
ros unless either GCC feels like implementing or we switch to a decent
compiler... I don't really like these macros either, especially because
of their limitations. But better than nothing.

- Thomas

[Alex Ionescu]

Gunnar Dalsnes wrote:

>   Ge van Geldorp wrote:
>
> > >   From:
> > > Gunnar Dalsnes
> > >
> > >
> > > >   And I think these macro's are a perfect
> > > > example of Phillip's point. 
> > > >  I have no idea how the flow of
> > > >  control is without looking at the macro definitions. 
> > >
> > >  Sure you do, if you try _reeeealy_ hard;-P 
> >
> >
> >  No, really, I don't <<without looking at the macro definitions>>. RETURN
> >  sounds much like return, it is non-obvious that they're actually 
> >  goto's to
> >  CLEANUP. Ofcourse, I figured it out when you committed that stuff 3 
> >  weeks
> >  ago, but when looking at it last night it was again non-obvious to me.
> >  On the other hand, I had no problem whatsoever figuring out the 
> >  macro-free
> >  code that Nathan posted: 
>
>  Yes, but how is this different from someone not knowing/understanding 
>  that a finally block is called when returning from a try block? 
That's a compiler language feature. That's like saying that learning 
some 3rd party macro is equivalent to what operator new does in C++.

>   I may very well think the the finally block is only
> executed if i run 
>  at the end of the try block.  
Someone that has learnt Win32 C/C++ programming and exception handling 
wouldn't think that.

>   But i _learned_ and figured out how it works. And now
> i _remeber_.  
I also learnt and remember English. But I chose not to learn Zimbabwean.

>   But its not that same you say, because the macro _can_
> be implemented 
>  by hardcoding, while try/finally cannot. Uhm, try/finally in ros IS 
>  macros;-P Noone said, "kjk, s*rew you and your seh macros." "This 
>  belongs in the compiler." "I refuse to learn how to use those ugly seh 
>  macros." 
I think KJK has told me a million times how ugly PSEH is and how it 
should be in the compiler. But unlike other macros, we desperately need 
PSEH macros, we don't have a way around it. And their flow-control is as 
"hidden" as the seh intrinsics in compiler SEH.

>  
> >  
> >  BOOL NtFunc()
> >  {
> >    BOOL bResult;
> >    void *pPointer = NULL;
> >
> >    Lock();
> >
> >    if (Stuff)
> >    {
> >      bResult = FALSE;
> >      goto cleanup;
> >    }
> >    ....
> >
> >    bResult = TRUE;
> >
> >  cleanup:
> >    if (pPointer)
> >      free(pPointer);
> >    Unlock(stuff);
> >    DPRINT1("NtFunc returned %i\n", bResult);
> >    return bResult;
> >  }
> >
> >
> > >   2)Using gotos are much more ugly imo. 
> >
> >
> >  Oh, so goto's are acceptable if and only if you hide them out of sight? 
>
>  No, i think gotos are ok internally but i dont like them for return. 
>  First set a retval and then goto to the end. ugh...ly.  
I think it's a great way to do

S = Foo();
if (S)
{
    S = Foo();
    if (S)
    {
      S = Foo();
        if (S)
        {
          S = Foo();
        }
        else
        {
          goto cleanup;
        }
    }
    else
    {
      goto cleanup;
    }
}
else
{
goto cleanup;
}

cleanup:
   HeapFree(...)
   return Status;

instead of having the cleanup code quadriplicated.

>   I didnt make those macros so i could type less. I made
> they so i can 
>  _read_ less. Thats the point. Readability. When looking at code i like 
>  to quickly spot the points of return. In complex code, and if it 
>  already have gotos, its confusing. Having a reserved word like RETURN 
>  is also nice for sytax highlightning (and its actually the same _word_ 
>  as normally used for return;-). Making up a mind about what the code 
>  does very quickly is nice, and with RETURN i can do that just as fast 
>  (faster) as with return.
>
> >   
>  Im sure all of you would like those macros if you didnt refuse to 
>  learn- and use them. But as long as you do you will off course hate 
>  them. I hate all the stuff i dont understand as well. 
That's really a flawed statement. Learning and using these macros won't 
change their inner deficiencies as being flow control macros. Learning 
and using them will just propagate a frowned-upon programming practice. 
Your argument is much like saying "I'm sure if you all used 
uninitialized variables you'll like them".

>  
>  G.  

Best regards,
Alex Ionescu

[crashfourit]

Hi, I'm new here, if y'all don't mind I'll throw my thoughts in. I tend 
to do soemthing like this:

int foo(void){
    int ret;

    if (stuf){
        ret = FALSE;
        goto error;
    };
    error:
    /* Error clean up code */

    done:
    return (ret);
};

I think that too many marcos can clutter up code. How ever, the marcos 
can be useful from keeping one from repeating the same task again and 
again, but one has to name them so other people can tell what they do.

Alex Ionescu wrote:

>   Gunnar Dalsnes wrote:
>
> >   Ge van Geldorp wrote:
> >
> > > >   From:
> > > > Gunnar Dalsnes
> > > >
> > > >
> > > > > And I think these macro's are a perfect example of Phillip's 
> > > > > point. I have no idea how the flow of
> > > > > control is without looking at the macro definitions.
> > > >
> > > >
> > > >
> > > >  Sure you do, if you try _reeeealy_ hard;-P 
> > >
> > >
> > >
> > >  No, really, I don't <<without looking at the macro definitions>>. 
> > >  RETURN
> > >  sounds much like return, it is non-obvious that they're actually 
> > >  goto's to
> > >  CLEANUP. Ofcourse, I figured it out when you committed that stuff 3 
> > >  weeks
> > >  ago, but when looking at it last night it was again non-obvious to me.
> > >  On the other hand, I had no problem whatsoever figuring out the 
> > >  macro-free
> > >  code that Nathan posted: 
> >
> >
> >  Yes, but how is this different from someone not knowing/understanding 
> >  that a finally block is called when returning from a try block? 
>
>  That's a compiler language feature. That's like saying that learning 
>  some 3rd party macro is equivalent to what operator new does in C++.
>
> >   I may very well think the the finally block is
> > only executed if i run 
> >  at the end of the try block.  
>
>  Someone that has learnt Win32 C/C++ programming and exception handling 
>  wouldn't think that.
>
> >   But i _learned_ and figured out how it works. And
> > now i _remeber_.  
>
>  I also learnt and remember English. But I chose not to learn Zimbabwean.
>
> >   But its not that same you say, because the macro
> > _can_ be implemented 
> >  by hardcoding, while try/finally cannot. Uhm, try/finally in ros IS 
> >  macros;-P Noone said, "kjk, s*rew you and your seh macros." "This 
> >  belongs in the compiler." "I refuse to learn how to use those ugly 
> >  seh macros." 
>
>  I think KJK has told me a million times how ugly PSEH is and how it 
>  should be in the compiler. But unlike other macros, we desperately 
>  need PSEH macros, we don't have a way around it. And their 
>  flow-control is as "hidden" as the seh intrinsics in compiler SEH.
>
> >  
> > >  
> > >  BOOL NtFunc()
> > >  {
> > >    BOOL bResult;
> > >    void *pPointer = NULL;
> > >
> > >    Lock();
> > >
> > >    if (Stuff)
> > >    {
> > >      bResult = FALSE;
> > >      goto cleanup;
> > >    }
> > >    ....
> > >
> > >    bResult = TRUE;
> > >
> > >  cleanup:
> > >    if (pPointer)
> > >      free(pPointer);
> > >    Unlock(stuff);
> > >    DPRINT1("NtFunc returned %i\n", bResult);
> > >    return bResult;
> > >  }
> > >
> > >
> > > >   2)Using gotos are much more ugly imo. 
> > >
> > >
> > >
> > >  Oh, so goto's are acceptable if and only if you hide them out of sight? 
> >
> >
> >  No, i think gotos are ok internally but i dont like them for return. 
> >  First set a retval and then goto to the end. ugh...ly.  
>
>  I think it's a great way to do
>
>  S = Foo();
>  if (S)
>  {
>     S = Foo();
>     if (S)
>     {
>       S = Foo();
>         if (S)
>         {
>           S = Foo();
>         }
>         else
>         {
>           goto cleanup;
>         }
>     }
>     else
>     {
>       goto cleanup;
>     }
>  }
>  else
>  {
>  goto cleanup;
>  }
>
>  cleanup:
>    HeapFree(...)
>    return Status;
>
>  instead of having the cleanup code quadriplicated.
>
> >   I didnt make those macros so i could type less. I
> > made they so i can 
> >  _read_ less. Thats the point. Readability. When looking at code i 
> >  like to quickly spot the points of return. In complex code, and if it 
> >  already have gotos, its confusing. Having a reserved word like RETURN 
> >  is also nice for sytax highlightning (and its actually the same 
> >  _word_ as normally used for return;-). Making up a mind about what 
> >  the code does very quickly is nice, and with RETURN i can do that 
> >  just as fast (faster) as with return.
> >
> > >   
> >  Im sure all of you would like those macros if you didnt refuse to 
> >  learn- and use them. But as long as you do you will off course hate 
> >  them. I hate all the stuff i dont understand as well. 
>
>  That's really a flawed statement. Learning and using these macros 
>  won't change their inner deficiencies as being flow control macros. 
>  Learning and using them will just propagate a frowned-upon programming 
>  practice. Your argument is much like saying "I'm sure if you all used 
>  uninitialized variables you'll like them".
>
> >  
> >  G.  
>
>
>  Best regards,
>  Alex Ionescu
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.com
>  http://reactos.com:8080/mailman/listinfo/ros-dev
>

[Gunnar Dalsnes]

> >   Yes, but how
> > is this different from someone not knowing/understanding 
> >  that a finally block is called when returning from a try block? 
>  
>  
>  That's a compiler language feature. That's like saying that learning 
>  some 3rd party macro is equivalent to what operator new does in C++. 
The new operator is just a word as anything else. Just because its a 
compiler feature doesnt make it "magic". It just means that every C++ 
compiler should reserve and support it just like we can say ReactOS 
reserve and support Xxx for usage Xxx. And you can overload new u know. 
Then you never know what it _really_ does;-P

>    
>    
> I also learnt and remember English. But I
chose not to learn Zimbabwean.

It this context it would be "Zimbabwean sound so weird I refuse to learn 
it. Zimbabwean is flawed and ppl should stop speaking it. They should 
learn English instead so I can understand them."

>    
>    
> instead of having the cleanup code
quadriplicated.

At least we agree on something.

So then Im free to apply that schema thruout ros? Or will I then get: 
"gotos sux", "please dont do this", "it looks so ugly",
"i refuse to do 
it this way" etc?

>    
>   
> That's really a flawed statement. Learning and using these macros won't

> change their inner deficiencies as being flow control macros.

They are just as deficient as the goto example you showed ei. equivalent.

>   Learning 
>  and using them will just propagate a frowned-upon programming practice. 
Just because someone else says you should do so doesnt make it right. 
Thinking for urself (open mind) and not caring about what others says 
("the standard") can be a relief.

>   Your argument is much like saying "I'm sure
> if you all used 
>  uninitialized variables you'll like them". 
It depends. In Java they would;-P

>    
>>

G.

[Richard Campbell]

Hate to throw my 2 cents in, but why use macros or goto statements at 
all?  None of the demonstrated code actually needs a goto statement to 
work.  Granted i've not seen the actual offensive code , but all 
examples here can be written without goto statements or macros.  Why 
bother using either?  At any rate, i'm inclined to agree that macros are 
a bad idea.  Hiding a mess behind a preprocessor is considered bad 
coding practice.

Richard

Gunnar Dalsnes wrote:

>  
> > >   Yes, but
> > > how is this different from someone not 
> > >  knowing/understanding that a finally block is called when returning 
> > >  from a try block? 
> >
> >
> >  That's a compiler language feature. That's like saying that learning 
> >  some 3rd party macro is equivalent to what operator new does in C++. 
>
>  The new operator is just a word as anything else. Just because its a 
>  compiler feature doesnt make it "magic". It just means that every C++ 
>  compiler should reserve and support it just like we can say ReactOS 
>  reserve and support Xxx for usage Xxx. And you can overload new u 
>  know. Then you never know what it _really_ does;-P
>
> >   
> >   
> > I also learnt and remember English.
> But I chose not to learn Zimbabwean.
>
>
>  It this context it would be "Zimbabwean sound so weird I refuse to 
>  learn it. Zimbabwean is flawed and ppl should stop speaking it. They 
>  should learn English instead so I can understand them."
>
> >   
> >   
> > instead of having the cleanup code
> quadriplicated.
>
>
>  At least we agree on something.
>
>  So then Im free to apply that schema thruout ros? Or will I then get: 
>  "gotos sux", "please dont do this", "it looks so ugly",
> "i refuse to 
>  do it this way" etc?
>
> >   
> >  
> > That's really a flawed statement. Learning and using these macros 
> > won't change their inner deficiencies as being flow control macros.
>
>
>  They are just as deficient as the goto example you showed ei. equivalent.
>
> >   Learning and using them will just propagate a
> > frowned-upon 
> >  programming practice. 
>
>  Just because someone else says you should do so doesnt make it right. 
>  Thinking for urself (open mind) and not caring about what others says 
>  ("the standard") can be a relief.
>
> >   Your argument is much like saying "I'm
> > sure if you all used 
> >  uninitialized variables you'll like them". 
>
>  It depends. In Java they would;-P
>
> >   
> >
> >  
>  G.
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.com
>  http://reactos.com:8080/mailman/listinfo/ros-dev
>

[Gunnar Dalsnes]

Richard Campbell wrote:
>   Hate to throw my 2 cents in, but why use macros or
> goto statements at 
>  all?  None of the demonstrated code actually needs a goto statement to 
>  work.  
Having common cleanup is _very_ good programming practice, and in lack 
of working/efficient try/finally i mean that using gotos and/or macros 
for this highy outweights the (alleged) bad practice of using gotos 
and/or macros.

>   Granted i've not seen the actual offensive code ,
> but all 
>  examples here can be written without goto statements or macros.  Why 
>  bother using either? 
Have a look at any medium to large function in ros and youll pretty fast 
find points of return (in case of error/fail) where it fails to do some 
cleanup. Having a common place for cleanup makes it easier to make this 
right.

Example: kernel32\process\create.c:BasepInitializeEnvironment().
return at line 496 & 574: fail to destroy proces params


>   At any rate, i'm inclined to agree that macros are
>
>  a bad idea.  Hiding a mess behind a preprocessor is considered bad 
>  coding practice. 
Thats exactly what macros _should_ be used for. Hiding mess. Im sure 
macros, gotos etc. can be abused but imo this is not the case here.

G.

[Richard Campbell]

And there are methods other then using goto that allow for common 
cleanup.  One such example is:

int somefunction()
{
    BOOL bSuccess;

    if ( someotherfunction(arg) )
    {
        if ( yetanotherfunction(arg) )
        {
            bSuccess = TRUE;
        }
    }
    CleanUpCode;
}

Anyways, back to the point.  Most devs have already argued against the 
idea of using macros.  I say the discussion stops and we call a vote.

Gunnar Dalsnes wrote:

>  
>  Richard Campbell wrote:
>
> >   Hate to throw my 2 cents in, but why use macros
> > or goto statements at 
> >  all?  None of the demonstrated code actually needs a goto statement 
> >  to work.  
>
>  Having common cleanup is _very_ good programming practice, and in lack 
>  of working/efficient try/finally i mean that using gotos and/or macros 
>  for this highy outweights the (alleged) bad practice of using gotos 
>  and/or macros.
>
> >   Granted i've not seen the actual offensive
> > code , but all examples 
> >  here can be written without goto statements or macros.  Why bother 
> >  using either? 
>
>  Have a look at any medium to large function in ros and youll pretty 
>  fast find points of return (in case of error/fail) where it fails to 
>  do some cleanup. Having a common place for cleanup makes it easier to 
>  make this right.
>
>  Example: kernel32\process\create.c:BasepInitializeEnvironment().
>  return at line 496 & 574: fail to destroy proces params
>
>
> >   At any rate, i'm inclined to agree that
> > macros are a bad idea.  
> >  Hiding a mess behind a preprocessor is considered bad coding practice. 
>
>  Thats exactly what macros _should_ be used for. Hiding mess. Im sure 
>  macros, gotos etc. can be abused but imo this is not the case here.
>
>  G.
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.com
>  http://reactos.com:8080/mailman/listinfo/ros-dev
>

[TwoTailedFox]

I say Majority Rules, and the code is reverted. But that's me.

On 9/28/05, Richard Campbell <eek2121(a)comcast.net> wrote:
>   And there are methods other then using goto that allow
> for common
>  cleanup.  One such example is:
>
>  int somefunction()
>  {
>      BOOL bSuccess;
>
>      if ( someotherfunction(arg) )
>      {
>          if ( yetanotherfunction(arg) )
>          {
>              bSuccess = TRUE;
>          }
>      }
>      CleanUpCode;
>  }
>
>  Anyways, back to the point.  Most devs have already argued against the
>  idea of using macros.  I say the discussion stops and we call a vote.
>
>  Gunnar Dalsnes wrote:
>
> >  
> >  Richard Campbell wrote:
> >
> > >   Hate to throw my 2 cents in, but why use macros
> > > or goto statements at
> > >  all?  None of the demonstrated code actually needs a goto statement
> > >  to work. 
> >
> >  Having common cleanup is _very_ good programming practice, and in lack
> >  of working/efficient try/finally i mean that using gotos and/or macros
> >  for this highy outweights the (alleged) bad practice of using gotos
> >  and/or macros.
> >
> > >   Granted i've not seen the actual offensive
> > > code , but all examples
> > >  here can be written without goto statements or macros.  Why bother
> > >  using either? 
> >
> >  Have a look at any medium to large function in ros and youll pretty
> >  fast find points of return (in case of error/fail) where it fails to
> >  do some cleanup. Having a common place for cleanup makes it easier to
> >  make this right.
> >
> >  Example: kernel32\process\create.c:BasepInitializeEnvironment().
> >  return at line 496 & 574: fail to destroy proces params
> >
> >
> > >   At any rate, i'm inclined to agree that
> > > macros are a bad idea.
> > >  Hiding a mess behind a preprocessor is considered bad coding practice. 
> >
> >  Thats exactly what macros _should_ be used for. Hiding mess. Im sure
> >  macros, gotos etc. can be abused but imo this is not the case here.
> >
> >  G.
> >  _______________________________________________
> >  Ros-dev mailing list
> >  Ros-dev(a)reactos.com
> >  http://reactos.com:8080/mailman/listinfo/ros-dev
> >  
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.com
>  http://reactos.com:8080/mailman/listinfo/ros-dev
>  

--
"I had a handle on life, but then it broke"

[Alex Ionescu]

TwoTailedFox wrote:

>  I say Majority Rules, and the code is reverted. But
> that's me.
>   
>  
I'm going to revert the macros in the code I maintain sometime this 
week... I've been working on getting Winsock 2.2 running so I haven't 
had much time.

Best regards,
Alex Ionescu

[Casper Hornstrup]

>   -----Original Message-----
>  From: ros-dev-bounces(a)reactos.com [mailto:[e-mail]] On Behalf Of Alex
> Ionescu
>  Sent: 29. september 2005 00:13
>  To: TwoTailedFox; ReactOS Development List
>  Subject: Re: [ros-dev] Re: [ros-svn] [gdalsnes]18113:-reorderInsertXscendingOrder macro
> argument
>  order and update uses
>  
>  TwoTailedFox wrote:
>  
> >  I say Majority Rules, and the code is reverted.
> > But that's me.
> >
> >  
>  I'm going to revert the macros in the code I maintain sometime this
>  week... I've been working on getting Winsock 2.2 running so I haven't
>  had much time.
>  
>  Best regards,
>  Alex Ionescu 
No, that would leed to even more inconsistencies in ReactOS. We
should vote on the issue and take appropriate action depending
on the result of the voting.

Casper

[Alex Ionescu]

Casper Hornstrup wrote:

>  
>
> No, that would leed to even more inconsistencies in ReactOS. We
> should vote on the issue and take appropriate action depending
> on the result of the voting.
>
> Casper
>
>
>   
>  
Go ahead and have your vote, but I'm simply going to overwrite their 
usage in my code when I commit new stuff.
You can't go ahead, piss on someone's code, and then have a vote if the 
person that got pissed on should get his code back or not. What kind of 
screwed up system is that? Go count the threads itself, I've counted 15 
people who agreed that:

1) Macros are bad, especially in ntoskrnl
2) Overwriting someone's maintained code is bad.

Regardless of the outcome of this "vote", the 4 files that I wrote 
99.99% of will be returned to their original state by me, it's my right.

Best regards,
Alex Ionescu

[Casper Hornstrup]

>   -----Original Message-----
>  From: ros-dev-bounces(a)reactos.com [mailto:[e-mail]] On Behalf Of Alex
> Ionescu
>  Sent: 29. september 2005 19:02
>  To: ReactOS Development List
>  Subject: Re: [ros-dev] Re: [ros-svn][gdalsnes]18113:-reorderInsertXscendingOrder macro
> argument
>  orderand update uses
>  
>  Go ahead and have your vote, but I'm simply going to overwrite their
>  usage in my code when I commit new stuff.
>  You can't go ahead, piss on someone's code, and then have a vote if the
>  person that got pissed on should get his code back or not. What kind of
>  screwed up system is that? Go count the threads itself, I've counted 15
>  people who agreed that:
>  
>  1) Macros are bad, especially in ntoskrnl
>  2) Overwriting someone's maintained code is bad.
>   
Voting is the best decision making procedure we've found so far.
Overwriting someone else's code just because you don't like it is counter
productive. If there are arguments for not implementing something in a
particular way then provide those arguments, vote on the subject, and
document the result in wiki. Whenever you remove code that violates a rule
in the future, you only need to reference the relevant section in wiki to
have the "right" to remove the code. There is no need to fight over which
code is "best" or which code is more "right". The rule is in wiki and
it
applies until replaced by a new rule. If there is no formal rule, then
there is nothing stopping another developer from replacing your replaced
code. This could go on forever or until all developers, but one give up,
wasting a lot of the (already limited) resources of the project.

>   Regardless of the outcome of this "vote",
> the 4 files that I wrote
>  99.99% of will be returned to their original state by me, it's my right. 
ReactOS has collective code ownership. This implies that all developers own
all code and you have no more right to change a piece of code than anyone
else which is taking part of the project. If you want to see module ownership
instead then propose a solution for this and call for a vote on the
subject.

Casper

[KJKHyperion]

Alex Ionescu wrote:

>   Go ahead and have your vote, but I'm simply going
> to overwrite their 
>  usage in my code when I commit new stuff. 
"Revert wars: not just for Wikipedia anymore"

[TwoTailedFox]

Revert Wars, ironically, end after a vote on Wikipedia ;)

On 9/29/05, KJKHyperion <hackbunny(a)reactos.com> wrote:
>   Alex Ionescu wrote:
>
> >   Go ahead and have your vote, but I'm simply
> > going to overwrite their
> >  usage in my code when I commit new stuff. 
>  "Revert wars: not just for Wikipedia anymore"
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev
>  

--
"I had a handle on life, but then it broke"

[Rick Langschultz]

I have a great question or solution to the problem:

Why not compare the two versions of the code. Test for speed and  
stability and then use which ever one is stable and fast. The more  
someone bickers about a particular subject the more won't get done.  
There are lots of different things people can be doing besides  
senseless arguing on the mailing list.

I have been working on implementing system services for a while now,  
kind of stuck but it should pick up here in a little bit.
On Sep 29, 2005, at 2:59 PM, TwoTailedFox wrote:

>   Revert Wars, ironically, end after a vote on Wikipedia
> ;)
>
>  On 9/29/05, KJKHyperion <hackbunny(a)reactos.com> wrote:
>
> >   Alex Ionescu wrote:
> >
> >
> > >   Go ahead and have your vote, but I'm simply
> > > going to overwrite their
> > >  usage in my code when I commit new stuff.
> > >  
> >  "Revert wars: not just for Wikipedia anymore"
> >  _______________________________________________
> >  Ros-dev mailing list
> >  Ros-dev(a)reactos.org
> >  http://www.reactos.org/mailman/listinfo/ros-dev
> >
> >  
>
>  --
>  "I had a handle on life, but then it broke"
>
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev
>  
Rick Langschultz
rlangschultz(a)cox.net (Home)
rlangschultz(a)ellemaespa.com (Work)
rlangschultz(a)email.uophx.edu (School)

[Gunnar Dalsnes]

> >    
> >  
>  Go ahead and have your vote, but I'm simply going to overwrite their 
>  usage in my code when I commit new stuff.
>  You can't go ahead, piss on someone's code 
Addition of the macros was done as an improvement, not to piss on (your) 
code. I just started at a random place in ntoskrnl (io) and took most 
files there.

G.

[crashofurit]

My question is: Why repeat the error exit code again and again? If used
wisely, using goto statements can reduce the in a function by a few bytes
and reduce the number of points of change. Intead of having to look for all
the error code scattered in a function, the error code is in one spot where
you don't have to change sevel lines of code scadered throughout a fuction
just to change the exiting behavior.
 "Crashfourit"
 On 9/28/05, Richard Campbell <eek2121(a)comcast.net> wrote:
>  
>  Hate to throw my 2 cents in, but why use macros or goto statements at
>  all? None of the demonstrated code actually needs a goto statement to
>  work. Granted i've not seen the actual offensive code , but all
>  examples here can be written without goto statements or macros. Why
>  bother using either? At any rate, i'm inclined to agree that macros are
>  a bad idea. Hiding a mess behind a preprocessor is considered bad
>  coding practice.
>
>  Richard
>
>  Gunnar Dalsnes wrote:
>
> >  
> > > >   Yes, but
> > > > how is this different from someone not
> > > >  knowing/understanding that a finally block is called when returning
> > > >  from a try block? 
> > >
> > >
> > >  That's a compiler language feature. That's like saying that learning
> > >  some 3rd party macro is equivalent to what operator new does in C++. 
> >
> >  The new operator is just a word as anything else. Just because its a
> >  compiler feature doesnt make it "magic". It just means that every C++
> >  compiler should reserve and support it just like we can say ReactOS
> >  reserve and support Xxx for usage Xxx. And you can overload new u
> >  know. Then you never know what it _really_ does;-P
> >
> > >   
> > >   
> > > I also learnt and remember English.
> > But I chose not to learn 
>  Zimbabwean.
> >  
> >
> >  It this context it would be "Zimbabwean sound so weird I refuse to
> >  learn it. Zimbabwean is flawed and ppl should stop speaking it. They
> >  should learn English instead so I can understand them."
> >
> > >   
> > >   
> > > instead of having the cleanup code
> > quadriplicated.
> >
> >
> >  At least we agree on something.
> >
> >  So then Im free to apply that schema thruout ros? Or will I then get:
> >  "gotos sux", "please dont do this", "it looks so ugly",
> > "i refuse to
> >  do it this way" etc?
> >
> > >   
> > >  
> > > That's really a flawed statement. Learning and using these macros
> > > won't change their inner deficiencies as being flow control macros.
> >
> >
> >  They are just as deficient as the goto example you showed ei. 
>  equivalent.
> >  
> > >   Learning and using them will just propagate a
> > > frowned-upon
> > >  programming practice. 
> >
> >  Just because someone else says you should do so doesnt make it right.
> >  Thinking for urself (open mind) and not caring about what others says
> >  ("the standard") can be a relief.
> >
> > >   Your argument is much like saying "I'm
> > > sure if you all used
> > >  uninitialized variables you'll like them". 
> >
> >  It depends. In Java they would;-P
> >
> > >  
> > > > 
> >  G.
> >  _______________________________________________
> >  Ros-dev mailing list
> >  Ros-dev(a)reactos.com
> >  http://reactos.com:8080/mailman/listinfo/ros-dev
> >  
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.com
>  http://reactos.com:8080/mailman/listinfo/ros-dev
>  


--
<P>My DeviantArt.com page: <A href="http://crashfourit.deviantart.com/"
>
http://crashfourit.deviantart.com/</A><BR>My FanFiction.net bio page: <A
href="http://www.fanfiction.net/u/726606/" >
http://www.fanfiction.net/u/726606/</A><BR>My Blog: <A
href="http://crashfourit.blogspot.com"
> http://crashfourit.blogspot.com</A><BR>America's Debate: <A
href="http://www.americasdebate.com/"
>  http://www.americasdebate.com/</A> </P>

[Nathan Woods]

Richard Campbell writes: 

>   Hate to throw my 2 cents in, but why use macros or
> goto statements at all? 
>   None of the demonstrated code actually needs a goto statement to work.  
>  Granted i've not seen the actual offensive code , but all examples here 
>  can be written without goto statements or macros.  Why bother using 
>  either?  At any rate, i'm inclined to agree that macros are a bad idea.  
>  Hiding a mess behind a preprocessor is considered bad coding practice. 
While the previous examples can ge accomplished cleanly without gotos and/or 
macros, the rules can be different when the code is far more complicated.  
For instance, the handling of error conditions buried deep within multiple 
for loops and other statements.  Take the following example: 


BOOL NtFunc()
{
   BOOL bSuccess = FALSE;
   LPVOID *pPointer = NULL;
   LPVOID *pAnotherPointer = NULL; 


   for (...)
   {
       if (ThisOperationCanFail())
           goto cleanup; 


       pPointer = malloc(...);
       if (!pPointer)
            goto cleanup; 


       while (...)
       {
           if (ThisOperationCanFailAlso())
              goto cleanup;
           ....
           if (!pAnotherPointer)
           {
              pAnotherPointer = malloc(...);
              if (!pPointer)
                 goto cleanup; 

              if (YetAnotherCanFail())
                  goto cleanup;
              ...
           }
       }
       free(pPointer);
       pPointer = NULL;
   }
   bSuccess = TRUE;
cleanup:
   if (pPointer)
       free(pPointer);
   if (pAnotherPointer)
       free(pAnotherPointer);
   return bSuccess;
} 

 

pPointer and pAnotherPointer are both pointers with different lifetimes; 
making things a bit messy.  While it is possible to write such code without 
gotos or macros, it would involve checking and freeing those pointers from 
many different places.  Using a cleanup label provides a convenient place to 
centrally locate all cleanup code. 


 -N

[Joseph Galbraith]

Nathan Woods wrote:
>   Richard Campbell writes:
> >   Hate to throw my 2 cents in, but why use macros
> > or goto statements at 
> >  all?  None of the demonstrated code actually needs a goto statement to 
> >  work.  Granted i've not seen the actual offensive code , but all 
> >  examples here can be written without goto statements or macros.  Why 
> >  bother using either?  At any rate, i'm inclined to agree that macros 
> >  are a bad idea.  Hiding a mess behind a preprocessor is considered bad 
> >  coding practice. 
>  
>  While the previous examples can ge accomplished cleanly without gotos 
>  and/or macros, the rules can be different when the code is far more 
>  complicated.  For instance, the handling of error conditions buried deep 
>  within multiple for loops and other statements.  Take the following 
>  example:
>  
>  BOOL NtFunc()
>  {
>    BOOL bSuccess = FALSE;
>    LPVOID *pPointer = NULL;
>    LPVOID *pAnotherPointer = NULL;
>  
>    for (...)
>    {
>        if (ThisOperationCanFail())
>            goto cleanup;
>  
>        pPointer = malloc(...);
>        if (!pPointer)
>             goto cleanup;
>  
>        while (...)
>        {
>            if (ThisOperationCanFailAlso())
>               goto cleanup;
>            ....
>            if (!pAnotherPointer)
>            {
>               pAnotherPointer = malloc(...);
>               if (!pPointer)
>                  goto cleanup;
>               if (YetAnotherCanFail())
>                   goto cleanup;
>               ...
>            }
>        }
>        free(pPointer);
>        pPointer = NULL;
>    }
>    bSuccess = TRUE;
>  cleanup:
>    if (pPointer)
>        free(pPointer);
>    if (pAnotherPointer)
>        free(pAnotherPointer);
>    return bSuccess;
>  }
>  
>  
>  pPointer and pAnotherPointer are both pointers with different lifetimes; 
>  making things a bit messy.  While it is possible to write such code 
>  without gotos or macros, it would involve checking and freeing those 
>  pointers from many different places.  Using a cleanup label provides a 
>  convenient place to centrally locate all cleanup code. 
Of course the real solution to this problem is the following
simple C++ class (which I'm not really arguing we should use...
I'm just throwing some gasoline on the flames :-)

template<class T>
class NPagedPointer
{
public:
     inline NPagedPointer(T* p) : m_ptr(p) {}
     inline ~NPagedPointer()
     {
        if ( m_ptr )
            ::ExFreePool(m_ptr);
     }

     inline operator T*() { return m_ptr; }
};

BOOL NtFunc()
{
   NPagedPointer<PLUGH> pAnotherPointer;

   for (...)
   {
     if (ThisOperationCanFail())
       return FALSE;

     NPagedPointer<XYZZY> pPointer = malloc(...);
     if ( ! pPointer )
       return FALSE;

     while (...)
     {
       if (ThisOperationCanFailAlso())
         return FALSE;

       ....

       if ( ! pAnotherPointer)
       {
         pAnotherPointer = malloc(...);
         if ( pAnotherPointer )
           return FALSE;
         if (YetAnotherCanFail())
           return FALSE;
         ...
       }
     }
   }

   return TRUE;
}

See, no cleanup code needed :-)

Of course, people don't like to use C++ in the kernel, and not
everyone has agreed to learn C++ (anymore than they've agreed to
learn Zimbobwean.)  And it can be tricky to make sure the C++
compiler doesn't mess you up (like putting the generated template
code in a paged section if you have one.)

Thanks,

Joseph

[Phillip Susi]

Gunnar Dalsnes wrote:
>   
>  The new operator is just a word as anything else. Just because its a 
>  compiler feature doesnt make it "magic". It just means that every C++ 
>  compiler should reserve and support it just like we can say ReactOS 
>  reserve and support Xxx for usage Xxx. And you can overload new u know. 
>  Then you never know what it _really_ does;-P
>   
I think you missed the point.  The fact of the matter is that new is a 
part of standard C++.  Every programmer who knows C++ can understand 
your code when you stick to standard C++.  When you start obfuscating 
the code with macros, it becomes much harder to understand, and in 
effect, you aren't really speaking C++ anymore because anyone fluent in 
C++ can't read your code.  It's like if you were to overload operator+ 
and have it do something completely unrelated to adding two things 
together.  Sure, you can do it, and sure, if you understand the 
implementation of that operator+ you can figure out what the code is 
doing, but it is in no way obvious to someone who knows C++ and has the 
expectation that an operator+ adds.  Hard to read code is bad code.

>   It this context it would be "Zimbabwean sound so
> weird I refuse to learn 
>  it. Zimbabwean is flawed and ppl should stop speaking it. They should 
>  learn English instead so I can understand them."
>   
Again you miss the point entirely.  I don't learn Zimbabwean because 
everyone I need to speak with understands English perfectly fine.  *YOU* 
speak English perfectly fine.  Just because you want to speak Zimbabwean 
does not mean that I should learn it.  Sure, I could, but why should I 
waste the time learning it when there is no gain?  You and I already can 
communicate in English, switching to Zimbabwean has no real benefit, so 
me learning it is only a waste of time.  You trying to FORCE me to learn 
it is well, insulting.


>   So then Im free to apply that schema thruout ros? Or
> will I then get: 
>  "gotos sux", "please dont do this", "it looks so ugly",
> "i refuse to do 
>  it this way" etc?
>   
Lots of code in ReactOS uses gotos in the way I described before.  As 
all beginning programming students are taught, gotos are evil, but 
sometimes they are the lesser evil.  This type of error handling in C is 
a time when CAREFUL use of gotos is less evil than the alternative.  It 
avoids duplicating cleanup code while remaining clear and concise, which 
is why it is an industry standard practice.  If you search you can find 
plenty of code out there that uses it, and plenty of best practices 
material that advises it.


>   
>  They are just as deficient as the goto example you showed ei. equivalent.
>  
No, they aren't.  The goto is defficient because it breaks the control 
of flow.  Exactly how it breaks it though, is obvious to anyone who 
speaks the language.  The alternative to breaking flow control is to 
duplicate cleanup code, and that is a greater evil bacuse it produces 
even harder to maintain code than using the gotos.  The macros not only 
break flow control, but they do so in a non obvious way, which makes 
them more evil than the gotos.  That increase in evil is not balenced by 
an increase in good, so it is a bad decision.

>   
>  Just because someone else says you should do so doesnt make it right. 
>  Thinking for urself (open mind) and not caring about what others says 
>  ("the standard") can be a relief.
>   

You seem to be missing the entire point of this conversation.  You are 
not a unique snowflake.  Being different for the sake of being different 
is not a good thing.  Ignoring others when you are trying to work with 
them is not a good thing.  You are not the first person to have this 
idea.  A great many before you have had this idea, and finally come to 
realize why it is bad.  That is the reason that use of gotos and not 
flow control macros have become industry standard.  While one should not 
blindly follow standards without question, there are very good reasons 
the standards exist, so unless you have a better reason not to follow 
it, you should.

In this case, following the generally accepted standards produces more 
readable code, and as a programmer, that should be one of your 
priorities.  Sometimes breaking with convention can be a good thing, but 
such a decision needs to be weighed carefully and taken only if the 
benefits outweigh the losses.  In this case, the only benefit to use of 
flow control macros is they save you a few keystrokes when writing the 
code, and you find it aesthetically pleasing.  Of course you do, you 
wrote it.

With no real benefit the needs of the many outweigh your aesthetic 
desire to use the macros.  The readability of the code to others and its 
maintainability in the future are far more important than your aesthetics.

>   Have a look at any medium to large function in ros and
> youll pretty
>  fast find points of return (in case of error/fail) where it fails to
>  do some cleanup. Having a common place for cleanup makes it easier to
>  make this right. 
Yes, having a common return point is a good programming practice, but 
again, when choosing HOW to have a common point of return, the method 
that is clearest and simplest and best understood produces the best code.

>   Thats exactly what macros _should_ be used for. Hiding
> mess. Im sure
>  macros, gotos etc. can be abused but imo this is not the case here. 
That IS the case here.  Macros should _ONLY_ be used to hide a mess when 
doing do has some pretty strong benefits to outweigh the complexity it 
adds.  In this case, the macro does not have any real benefit, therefore 
the added complexity with no benefit constitutes abuse.

[Gunnar Dalsnes]

Phillip Susi wrote:
>   Gunnar Dalsnes wrote:
>  
> >  
> >  The new operator is just a word as anything else. Just because its a 
> >  compiler feature doesnt make it "magic". It just means that every C++ 
> >  compiler should reserve and support it just like we can say ReactOS 
> >  reserve and support Xxx for usage Xxx. And you can overload new u 
> >  know. Then you never know what it _really_ does;-P
> >  
>  
>  I think you missed the point. 
You take this too serious. Didnt you see the smiley?

<snip>

>   Lots of code in ReactOS uses gotos in the way I
> described before.  As 
>  all beginning programming students are taught, gotos are evil, but 
>  sometimes they are the lesser evil.  This type of error handling in C is 
>  a time when CAREFUL use of gotos is less evil than the alternative.  It 
>  avoids duplicating cleanup code while remaining clear and concise, which 
>  is why it is an industry standard practice.  If you search you can find 
>  plenty of code out there that uses it, and plenty of best practices 
>  material that advises it. 
SOME functions does, but i can probably count them on one hand (you 
wrote those?). To beeing an industri standard as you claim it surely 
hasnt cought on.

<snip>

First, Im not forcing the RETURN macros on anyone (I am trying to force 
the list macros, but its for your own good;-P) I applied those RETURN 
macros in win32k as a part of a locking rewrite and until i myself 
presented them as an alternative to gotos, I havent heard a damn thing 
about it.

Again, here are the benefits with RETURN:
1) easy to use. looks like regular return but different enough to make 
people understand they are different;-P
2) much easier to read and spot exit points compared to gotos that jump 
to various return labels (done, error, cleanup, exit, cleanup2, cleanup3 
etc) mixed with "regular" gotos.
3) syntax highlight RETURN and you see a whole new world.
4) a proposed standard way of doing cleanup. not dozens of different 
goto labels (done, error, cleanup, exit, cleanup2, cleanup3 etc).

G.

[Ge van Geldorp]

>   From: Gunnar Dalsnes
>
> >   Oh, so goto's are acceptable if and only if
> > you hide them 
> >  out of sight? 
>  
>  No, i think gotos are ok internally but i dont like them for return. 
>  First set a retval and then goto to the end. ugh...ly. 
The definition of your RETURN():

#define RETURN(value) { _ret_ = value; goto _cleanup_; }

which seems to set a retval and then does a goto to the end. So, again,
"ugly code" (your words, not mine) is acceptable if you hide it out of
sight?
BTW, the following code which looks perfectly acceptable won't compile
correctly with your definition:

if (somecondition)
  RETURN(errorcode);
else
  do_something_else();

I admit this is a bit contrived example, you don't need the "else".

Gé van Geldorp.

[Gunnar Dalsnes]

Ge van Geldorp wrote:
> >  From: Gunnar
> > Dalsnes
> >
> >
> > >  Oh, so goto's are acceptable if and only if
> > > you hide them 
> > > out of sight? 
> > No, i think gotos are ok internally but i dont like them for return. 
> > First set a retval and then goto to the end. ugh...ly. 
>  
>  
>  The definition of your RETURN():
>  
>  #define RETURN(value) { _ret_ = value; goto _cleanup_; }
>  
>  which seems to set a retval and then does a goto to the end. So, again,
>  "ugly code" (your words, not mine) is acceptable if you hide it out of
>  sight? 
Yes. Hiding it _is_ the point. Readability is what its all about. Not 
the code behind the names. I dont care whats behind a name/operator 
(compiler builtin, assembly, function call, macro etc.) as long as it do 
what i want and have a (in this case) _uniqueue_ name that describes 
what it does. Imo, "RETURN(45)" look better than "ret = 45; goto
cleanup;".

BTW: As for thinking about what low level code lies behind a name i 
remeber the old(?) "i dont want to use while(TRUE) because it produce 
shitty asm, so i use for(;;) instead". I don't know if anyone thinks 
that anymore, but those who did needs a good spanking;-P

>   BTW, the following code which looks perfectly
> acceptable won't compile
>  correctly with your definition:
>  
>  if (somecondition)
>    RETURN(errorcode);
>  else
>    do_something_else();
>  
>  I admit this is a bit contrived example, you don't need the "else". 
Finally some constructive critisism;-P

It can be solved like this:

#define RETURN(value) do {_ret_ = value; goto _cleanup_; }while(0)

>   
>  Gé van Geldorp.
>   
G.

[Alex Ionescu]

Gunnar Dalsnes wrote:

>   Ge van Geldorp wrote:
>
> > >   From:
> > > Gunnar Dalsnes
> > >
> > >
> > > >   Oh, so goto's are acceptable if and only if
> > > > you hide them out of 
> > > >  sight? 
> > >
> > >  No, i think gotos are ok internally but i dont like them for return. 
> > >  First set a retval and then goto to the end. ugh...ly. 
> >
> >
> >  The definition of your RETURN():
> >
> >  #define RETURN(value) { _ret_ = value; goto _cleanup_; }
> >
> >  which seems to set a retval and then does a goto to the end. So, again,
> >  "ugly code" (your words, not mine) is acceptable if you hide it out of
> >  sight? 
>
>  Yes. Hiding it _is_ the point.  
You've just proven yourself what's wrong with flow control macros. Do 
you really think that in the sorry state in which Win32K is, it really 
needs macros to HIDE what's going on? When we can't even show a font 
properly with -O2? When we still have random graphics corruption? When 
we.. (I could go on for ages).

>   Readability is what its all about. 
Readability != Hiding code.

>   Not the code behind the names. I dont care whats
> behind a 
>  name/operator (compiler builtin, assembly, function call, macro etc.) 
>  as long as it do what i want and have a (in this case) _uniqueue_ name 
>  that describes what it does. Imo, "RETURN(45)" look better than "ret = 
>  45; goto cleanup;". 
"I"mo. Not the "o" of 15 other people. So don't force it.

>  
>  BTW: As for thinking about what low level code lies behind a name i 
>  remeber the old(?) "i dont want to use while(TRUE) because it produce 
>  shitty asm, so i use for(;;) instead". I don't know if anyone thinks 
>  that anymore, but those who did needs a good spanking;-P
>
> >   BTW, the following code which looks perfectly
> > acceptable won't compile
> >  correctly with your definition:
> >
> >  if (somecondition)
> >    RETURN(errorcode);
> >  else
> >    do_something_else();
> >
> >  I admit this is a bit contrived example, you don't need the "else". 
>
>  Finally some constructive critisism;-P
>
>  It can be solved like this:
>
>  #define RETURN(value) do {_ret_ = value; goto _cleanup_; }while(0)
>
> >  
> >  Gé van Geldorp.
> >  
>  G. 
Best regards,
Alex Ionescu

[Gunnar Dalsnes]

> >  
> >  Yes. Hiding it _is_ the point.  
>  
>  
>  You've just proven yourself what's wrong with flow control macros. Do 
>  you really think that in the sorry state in which Win32K is, it really 
>  needs macros to HIDE what's going on? When we can't even show a font 
>  properly with -O2? When we still have random graphics corruption? When 
>  we.. (I could go on for ages). 
I can't see how the poor state of win32k has anything to do with it. 
Returning from a function isnt exactly rocket science, and the amount of 
code and logic hidden is minimal.

>   
> >   Readability is what its all about. 
>  
>  
>  Readability != Hiding code. 
readability: writing (print or handwriting) that can be easily read.

Im pretty sure its easier to read something if its less to read. By your 
definition, asm would give total readability;-P

Not directly related, but moving some repeated code into a function is 
"hiding" code, and it makes the code more readable, reuseable and 
maintainable.

>    
G.

[KJKHyperion]

Gunnar Dalsnes wrote:

> >   You've
> > just proven yourself what's wrong with flow control macros. Do 
> >  you really think that in the sorry state in which Win32K is, it 
> >  really needs macros to HIDE what's going on? When we can't even show 
> >  a font properly with -O2? When we still have random graphics 
> >  corruption? When we.. (I could go on for ages). 
>  I can't see how the poor state of win32k has anything to do with it. 
>  Returning from a function isnt exactly rocket science, and the amount 
>  of code and logic hidden is minimal. 
I'm with Gunnar on this. It's a pattern (system calls in general are 
pattern-heavy), it's better to codify it. Microsoft would approve

[Phillip Susi]

This is your bad assumption.  I can write a sentence that is 12 words 
and 100 characters long, or I can write it using only 6 words that are 
50 characters long.  If those 6 words aren't in common usage, then most 
people won't understand it when they see it, and so it is harder to read 
even though it is shorter.

To show another example in C, I can write:

if( condition1 )
{
   dosomething1();
   if( condition2 )
     if( condition 3 )
       dosomething3();
     else dosomething2();
}

or I can write code that does the same thing, but is shorter:

condition1 && dosomething1(), condition3 ? dosomething3 : dosomething2();

Yes, the latter code is shorter, but the former is far easier to read 
and maintain.


To try and steer this back to the original issue, the LIST_FOR_EACH 
macro causes nonintuitive branching which is hard to understand when you 
read it, and when working on the code, as Alex said, you often have to 
demacrofy it which can introduce more errors.  These are legitimate down 
sides to using the macro, so again, you need a good reason to use it.

So far you seem to only keep insisting that the macros look better to 
you, so I say again, pleasing your sense of aesthetics does not outweigh 
the loss of maintainability.

Gunnar Dalsnes wrote:
>  
>  readability: writing (print or handwriting) that can be easily read.
>  
>  Im pretty sure its easier to read something if its less to read. By your 
>  definition, asm would give total readability;-P
>

[Gunnar Dalsnes]

Phillip Susi wrote:

>   To try and steer this back to the original issue, the
> LIST_FOR_EACH 
>  macro causes nonintuitive branching which is hard to understand when you 
>  read it 
Lerning it is well worth the effort.

>   and when working on the code, as Alex said, you often
> have to 
>  demacrofy it which can introduce more errors.   
This is the only valid point and to my defence i have said that some of 
the loops i replaced were for loops and here he would have exactly the 
same problem.

>   These are legitimate down 
>  sides to using the macro, so again, you need a good reason to use it.
>  
>  So far you seem to only keep insisting that the macros look better to 
>  you, so I say again, pleasing your sense of aesthetics does not outweigh 
>  the loss of maintainability. 
As said before, there really isnt a way to maintain walking a list. This 
is code printed in stone.

>    
G.

[Gunnar Dalsnes]

The list macros are widely used in both Wine (4 different macros) and in 
the linux kernel 
http://www.gelato.unsw.edu.au/~dsw/public-files/kernel-docs/kernel-api/r802… 
(i counted 16 different list walking macros).

A googled for a while looking for similar discussions about using such 
macros or not, but found none...

Seems like people who hate macros are the people who are drawn to 
reactos. I dont know what this means but it surely is weird. I hope it 
doesnt mean that reactos coders are too stupid to learn macros;-P

Well, at least i tried. Better luck next time (/me pat self on shoulder)

G.

[Alex Ionescu]

Gunnar Dalsnes wrote:

>   The list macros are widely used in both Wine (4
> different macros) and 
>  in the linux kernel 
>  http://www.gelato.unsw.edu.au/~dsw/public-files/kernel-docs/kernel-api/r802… 
>  (i counted 16 different list walking macros). 
I hope the Linux Kernel won't ever become an example of how an NT 
micro-kernel shoudl be written..

>  
>  A googled for a while looking for similar discussions about using such 
>  macros or not, but found none... 
Really? Let me point you to some of the famous threads by our Microsoft 
friends, including some of their most distinguished engineers:

http://blogs.msdn.com/larryosterman/archive/2004/12/01/273160.aspx

"

     Larry: You said Macros work to hide the complexity and say so like
    it is a bad thing.. ? Excuse me but I thought that was the POINT of
    using a Macro..

Actually, in the world in which I live (writing systems programs that 
exist in the working set of hundreds of millions of users), hiding 
complexity is a very, very bad thing.

You need to be VERY careful whenever you do something that hides 
complexity, because it's likely to come back and bite you on the behind."

Also: "Rules of software engineering" touches on this:
http://blogs.msdn.com/larryosterman/archive/2004/04/06/108686.aspx


>  
>  Seems like people who hate macros are the people who are drawn to reactos. 
Seems like people who hate macros are the people who are drawn to NT, 
too. I'm glad 99.9% of the developers here are on that line of though.

>   I dont know what this means but it surely is weird. I
> hope it doesnt 
>  mean that reactos coders are too stupid to learn macros;-P 
Yeah, go tell Larry Osterman he's too stupid to learn a macro.

>  
>  Well, at least i tried. Better luck next time (/me pat self on shoulder) 
Read Larry's posts. Read them again. Read the comments by other MS 
engineers. If you still think hiding complexity is a good thing, God 
help you.

>  
>  G.
>  
Best regards,
Alex Ionescu

[Bryan Quigley]

Hiding complexity is a good thing. It allows you to see the big picture. (As
long as its modularized).

I think you need to specify "hiding complexity in ...something... ", to make
this statement valid.

I don't see why you can't have a cleanup function, aka something like
CleanUp3Pointers(Ptr1,Ptr2,Ptr3).

DISCLAIMER: I'm in college, and might not know what I am talking about.

[Gunnar Dalsnes]

Alex Ionescu wrote:
>   Gunnar Dalsnes wrote:
>  
> >   The list macros are widely used in both Wine (4
> > different macros) and 
> >  in the linux kernel 
> >  http://www.gelato.unsw.edu.au/~dsw/public-files/kernel-docs/kernel-api/r802… 
> >  (i counted 16 different list walking macros). 
>  
>  
>  I hope the Linux Kernel won't ever become an example of how an NT 
>  micro-kernel shoudl be written.. 
Using macros or not really has nothing to do with the kernel design... 
Its about the people who code.

>   
> >  
> >  A googled for a while looking for similar discussions about using such 
> >  macros or not, but found none... 
>  
>  
>  Really? Let me point you to some of the famous threads by our Microsoft 
>  friends, including some of their most distinguished engineers:
>  
>  http://blogs.msdn.com/larryosterman/archive/2004/12/01/273160.aspx
>  
>  "
>  
>      Larry: You said Macros work to hide the complexity and say so like
>     it is a bad thing.. ? Excuse me but I thought that was the POINT of
>     using a Macro..
>  
>  Actually, in the world in which I live (writing systems programs that 
>  exist in the working set of hundreds of millions of users), hiding 
>  complexity is a very, very bad thing.
>  
>  You need to be VERY careful whenever you do something that hides 
>  complexity, because it's likely to come back and bite you on the behind."
>  
>  Also: "Rules of software engineering" touches on this:
>  http://blogs.msdn.com/larryosterman/archive/2004/04/06/108686.aspx
>   
This is not directly related to using list macros. I hate generic 
banning: "gotos considered harmful". bah... Also, for being careful, id 
say that the fact that Linux and Wine have used list macros extensively 
for ages and it works well, thats a pretty good pointer.

>   
> >  
> >  Seems like people who hate macros are the people who are drawn to 
> >  reactos. 
>  
>  
>  Seems like people who hate macros are the people who are drawn to NT, 
>  too. I'm glad 99.9% of the developers here are on that line of though.
>  
> >   I dont know what this means but it surely is
> > weird. I hope it doesnt 
> >  mean that reactos coders are too stupid to learn macros;-P 
>  
>  
>  Yeah, go tell Larry Osterman he's too stupid to learn a macro. 
The problem with MS and macros i can guess is: for foss projects, people 
who code are devoted to what they do and they only do it for fun (and 
recognizion). For a big corp that pays people to code, the devotion is 
probably not that high (not the fun either) and the flow of programmers 
is _much_ higher (not necesarely in and out of ms, but between different 
projects). Thats why i understand Larry. He want to protect the source 
agains incompetent programmers. I guess you could say that we have the 
same problem, we could get new incompetent members? Well, we can kick 
them hard and long, and also we monitor commits and we review the code.

>   
> >  
> >  Well, at least i tried. Better luck next time (/me pat self on shoulder) 
>  
>  
>  Read Larry's posts. Read them again. Read the comments by other MS 
>  engineers. If you still think hiding complexity is a good thing, God 
>  help you. 
Just because we are cloning thei OS doesnt mean we should clone their 
opinions, meanings and thoughts. There might still be hope for you thou;-P

G.

>   
> >  
> >  G.
> >  
>  Best regards,
>  Alex Ionescu
>  
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.com
>  http://reactos.com:8080/mailman/listinfo/ros-dev
>  
>  
>

[Gunnar Dalsnes]

>   Really? Let me point you to some of the famous threads
> by our Microsoft 
>  friends, including some of their most distinguished engineers:
>  
>  http://blogs.msdn.com/larryosterman/archive/2004/12/01/273160.aspx 
I read it, and there are just as many posts against his as with him.

Larrys "point" is that "one line of code (in this case a macro) 
generated 1kb of code and it made the binariy huge". Also, larry thinks 
using vector is just as bad, so you shouldnt code in standard C++, ever! 
C++ is banned also!

"/me: But Larry, i have a small macro and i _know_ it wont generate more 
code compared to manual writing. Its a real simple macro. Larry: Macros 
are evil i say! Never, ever use them! Never! /me: but Windows exes are 
alredy huge and Windows use lots of memory. It doesnt look like not 
using macros have helped for Windows? Larry: Uhm..."

A little puppet show for u there;-P

>    
G.

[Casper Hornstrup]

>   The alternative is: do the cleanup at every return,
> use goto or use
>  try/finally.
>  1)Cleanup at every return is madness. Most functions in ros do a large
>  amount of cleanup at each return and I sometimes spot mistakes where
>  one/more return misses some cleanup. Those errors are _hard_ to find. 
The functions are too large then. Use more smaller functions instead.

Casper

[crashofurit]

Using smaller functions instead when we are only using the code once, may
help on readability but can cost speed and space. (Think of what standard
code C uses to enter and exit a function.) But then again, you can get
around that by using declaring those functions as inline.

On 9/28/05, Casper Hornstrup <ch(a)csh-consult.dk> wrote:
>  
>
> >   The alternative is: do the cleanup at every
> > return, use goto or use
> >  try/finally.
> >  1)Cleanup at every return is madness. Most functions in ros do a large
> >  amount of cleanup at each return and I sometimes spot mistakes where
> >  one/more return misses some cleanup. Those errors are _hard_ to find. 
>  The functions are too large then. Use more smaller functions instead.
>
>  Casper
>
>
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.com
>  http://reactos.com:8080/mailman/listinfo/ros-dev
>  


--
<P>My DeviantArt.com page: <A
href="http://crashfourit.deviantart.com/">
http://crashfourit.deviantart.com/</A><BR>My FanFiction.net bio page: <A
href="http://www.fanfiction.net/u/726606/">
http://www.fanfiction.net/u/726606/</A><BR>My Blog: <A href="
http://crashfourit.blogspot.com">http://crashfourit.blogspot.com<…
Debate: <A href="http://www.americasdebate.com/">
http://www.americasdebate.com/</A> </P>

[Casper Hornstrup]

Oh please don't go there. Do you honestly think you (in general) are smarter at

generating microprocessor instructions than a multi-million hour compiler software?

If the execution speed is acceptable, I will always prefer readability of the source code

over execution speed. I don't really care if some function can be executed 1ns faster
if

several functions were collapsed into one.

 

If I could trade a 1 second added boot up time (~8 second total then?) for readable

ReactOS source code, I would do it immediately ;-)

 

Casper

 

  _____  

From: ros-dev-bounces(a)reactos.com [mailto:[e-mail]] On Behalf Of
crashofurit
Sent: 28. september 2005 17:22
To: ReactOS Development List
Subject: Re: [ros-dev] Re: [ros-svn] [gdalsnes]18113:-reorderInsertXscendingOrder macro
argument order andupdate uses

 

Using smaller functions instead when we are only using the code once, may help on
readability but can cost speed and space. (Think
of what standard code C uses to enter and exit a function.) But then again, you can get
around that by using declaring those
functions as inline. 

On 9/28/05, Casper Hornstrup <ch(a)csh-consult.dk> wrote: 


>   The alternative is: do the cleanup at every return,
> use goto or use
>  try/finally.
>  1)Cleanup at every return is madness. Most functions in ros do a large 
>  amount of cleanup at each return and I sometimes spot mistakes where
>  one/more return misses some cleanup. Those errors are _hard_ to find. 
The functions are too large then. Use more smaller functions instead. 

Casper


_______________________________________________
Ros-dev mailing list
Ros-dev(a)reactos.com
http://reactos.com:8080/mailman/listinfo/ros-dev




-- 
<P>My DeviantArt.com page: <A href="http://crashfourit.deviantart.com/"
>http://crashfourit.deviantart.com/</A><BR>My FanFiction.net
bio page: <A href="http://www.fanfiction.net/u/726606/"
>http://www.fanfiction.net/u/726606/</A><BR>My Blog: <A
href="http://crashfourit.blogspot.com"
>http://crashfourit.blogspot.com</A><BR>America's Debate: <A
href="http://www.americasdebate.com/"
>http://www.americasdebate.com/</A> </P>

[Thomas Weidenmueller]

Casper Hornstrup wrote:
>   Oh please don’t go there. Do you honestly think you
> (in general) are
>  smarter at generating microprocessor instructions than a multi-million hour
>  compiler software? 
In some places it does matter, especially in the kernel.

Splitting up a 200 lines functions in 10 helper functions IMO is ugly,
especially when those helper functions are only used by that one single
function. To avoid code duplication I *do* create helper functions, but
most often only for things that can be shared with other code. I don't
have problems with functions with a few hundreds of lines of code as
long as the coding style is acceptable and it is sufficiently documented
with comments.

However, we're going off topic...

- Thomas

[Casper Hornstrup]

>   -----Original Message-----
>  From: ros-dev-bounces(a)reactos.com [mailto:[e-mail]] On Behalf Of
> Thomas
>  Weidenmueller
>  Sent: 28. september 2005 18:15
>  To: ReactOS Development List
>  Subject: Re: [ros-dev] Re:[ros-svn] [gdalsnes]18113:-reorderInsertXscendingOrdermacro
> argument
>  order andupdate uses
>  
>  Casper Hornstrup wrote:
> >   Oh please don't go there. Do you honestly
> > think you (in general) are
> >  smarter at generating microprocessor instructions than a multi-million hour
> >  compiler software? 
>  In some places it does matter, especially in the kernel.
Prove it.

>   
>  Splitting up a 200 lines functions in 10 helper functions IMO is ugly,
>  especially when those helper functions are only used by that one single
>  function. To avoid code duplication I *do* create helper functions, but
>  most often only for things that can be shared with other code. I don't
>  have problems with functions with a few hundreds of lines of code as
>  long as the coding style is acceptable and it is sufficiently documented
>  with comments.
>  
>  However, we're going off topic...
>  
>  - Thomas 
Ok. I'll not argument against 200 line long functions then since it seems
I won't get anywhere.

Casper

[Jakob Eriksson]

Thomas Weidenmueller wrote:

>  Casper Hornstrup wrote:
>   
>
> >  Oh please don’t go there. Do you honestly think
> > you (in general) are
> > smarter at generating microprocessor instructions than a multi-million hour
> > compiler software?
> >     
> >  
> In some places it does matter, especially in the kernel.
>
> Splitting up a 200 lines functions in 10 helper functions IMO is ugly,
> especially when those helper functions are only used by that one single
> function. To avoid code duplication I *do* create helper functions, but
> most often only for things that can be shared with other code. I don't
> have problems with functions with a few hundreds of lines of code as
> long as the coding style is acceptable and it is sufficiently documented
> with comments.
>
> However, we're going off topic...
>   
>  
I think you are on topic.
These 10 helper functions usually make you avoid the gotos or the defines.
One mans ugly is anothers beauty.
Are 10 functions instead of one ugly?
Are macros?
Are gotos?

regards,
Jakob

[Thomas Weidenmueller]

Casper Hornstrup wrote:
>    
>  
> >  The alternative is: do the cleanup at every
> > return, use goto or use
> > try/finally.
> > 1)Cleanup at every return is madness. Most functions in ros do a large
> > amount of cleanup at each return and I sometimes spot mistakes where
> > one/more return misses some cleanup. Those errors are _hard_ to find. 
>  
>  
>  The functions are too large then. Use more smaller functions instead. 
I agree with Nathan. Having tons of small functions often isn't a good
solution, especially when you'd have to create dozens of small helper
functions. That not just only generates slower code but also makes it
more difficult to get a picture of the algorithm used. I much more
prefer jumping to cleanup labels the way Nathan demonstrated it. Of
course I avoid it where it doesn't make sense.

- Thomas

[crashofurit]

However an large function can also ramper readability and mantainiblility. I
think the best thing is to evaluate on how to write a function on a case by
case basis.

On 9/28/05, Thomas Weidenmueller <w3seek(a)reactos.com> wrote:
>  
>  Casper Hornstrup wrote:
> >  
> >
> > >  The alternative is: do the cleanup at every
> > > return, use goto or use
> > > try/finally.
> > > 1)Cleanup at every return is madness. Most functions in ros do a large
> > > amount of cleanup at each return and I sometimes spot mistakes where
> > > one/more return misses some cleanup. Those errors are _hard_ to find. 
> >
> >  The functions are too large then. Use more smaller functions instead. 
>  I agree with Nathan. Having tons of small functions often isn't a good
>  solution, especially when you'd have to create dozens of small helper
>  functions. That not just only generates slower code but also makes it
>  more difficult to get a picture of the algorithm used. I much more
>  prefer jumping to cleanup labels the way Nathan demonstrated it. Of
>  course I avoid it where it doesn't make sense.
>
>  - Thomas
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.com
>  http://reactos.com:8080/mailman/listinfo/ros-dev
>  


--
<P>My DeviantArt.com page: <A
href="http://crashfourit.deviantart.com/">
http://crashfourit.deviantart.com/</A><BR>My FanFiction.net bio page: <A
href="http://www.fanfiction.net/u/726606/">
http://www.fanfiction.net/u/726606/</A><BR>My Blog: <A href="
http://crashfourit.blogspot.com">http://crashfourit.blogspot.com<…
Debate: <A href="http://www.americasdebate.com/">
http://www.americasdebate.com/</A> </P>

[Casper Hornstrup]

>   -----Original Message-----
>  From: ros-dev-bounces(a)reactos.com [mailto:[e-mail]] On Behalf Of
> Thomas
>  Weidenmueller
>  Sent: 28. september 2005 17:27
>  To: ReactOS Development List
>  Subject: Re: [ros-dev] Re:[ros-svn] [gdalsnes] 18113:-reorderInsertXscendingOrdermacro
> argument
>  order and update uses
>  
>  Casper Hornstrup wrote:
> >  
> >
> > >  The alternative is: do the cleanup at every
> > > return, use goto or use
> > > try/finally.
> > > 1)Cleanup at every return is madness. Most functions in ros do a large
> > > amount of cleanup at each return and I sometimes spot mistakes where
> > > one/more return misses some cleanup. Those errors are _hard_ to find. 
> >
> >  The functions are too large then. Use more smaller functions instead. 
>  
>  I agree with Nathan. Having tons of small functions often isn't a good
>  solution, especially when you'd have to create dozens of small helper
>  functions. That not just only generates slower code but also makes it
>  more difficult to get a picture of the algorithm used. I much more
>  prefer jumping to cleanup labels the way Nathan demonstrated it. Of
>  course I avoid it where it doesn't make sense.
>  
>  - Thomas 
There is practically no difference in execution speed.

Casper

[Jakob Eriksson]

Take a look at the code below. More readable by far IMHO.



Casper Hornstrup wrote:

>    
>
> >  -----Original Message-----
> > From: ros-dev-bounces(a)reactos.com [mailto:[e-mail]] On Behalf Of
> > Thomas
> > Weidenmueller
> > Sent: 28. september 2005 17:27
> > To: ReactOS Development List
> > Subject: Re: [ros-dev] Re:[ros-svn] [gdalsnes] 18113:-reorderInsertXscendingOrdermacro
> > argument
> > order and update uses
> >
> > Casper Hornstrup wrote:
> >     
> >
> > >        
> > >
> > > >  The alternative is: do the cleanup at every
> > > > return, use goto or use
> > > > try/finally.
> > > > 1)Cleanup at every return is madness. Most functions in ros do a large
> > > > amount of cleanup at each return and I sometimes spot mistakes where
> > > > one/more return misses some cleanup. Those errors are _hard_ to find.
> > > >         
> > > >  
> > > The functions are too large then. Use more smaller functions instead.
> > >       
> > >  
> > I agree with Nathan. Having tons of small functions often isn't a good
> > solution, especially when you'd have to create dozens of small helper
> > functions. That not just only generates slower code but also makes it
> > more difficult to get a picture of the algorithm used. I much more
> > prefer jumping to cleanup labels the way Nathan demonstrated it. Of
> > course I avoid it where it doesn't make sense.
> >
> > - Thomas
> >     
> >  
> There is practically no difference in execution speed.
>
> Casper
>
>   
>
> ------------------------------------------------------------------------
>
> #include <stdio.h>
> #include <windows.h>
>
> #define NumberOfIterations	60000
>
> void RunCollapsed()
> {
> 	int i, j;
> 	for (i = 0; i < NumberOfIterations; i++)
> 	{
> 		for (j = 0; j < NumberOfIterations; j++)
> 		{
> 		}
> 	}
> }
>
> static void Helper()
> {
> 	int j;
> 	for (j = 0; j < NumberOfIterations; j++)
> 	{
> 	}
> }
>
> void RunNonCollapsed()
> {
> 	int i;
> 	for (i = 0; i < NumberOfIterations; i++)
> 	{
> 		Helper();
> 	}
> }
>
> void RunOnce(LARGE_INTEGER *collapsedTicks, LARGE_INTEGER *nonCollapsedTicks)
> {
> 	LARGE_INTEGER start;
> 	LARGE_INTEGER stop;
> 	
> 	QueryPerformanceCounter(&start);
> 	RunCollapsed();
> 	QueryPerformanceCounter(&stop);
> 	collapsedTicks->QuadPart = stop.QuadPart - start.QuadPart;
> 	printf("Collapsed: %lu ticks\n", collapsedTicks->QuadPart);
>
> 	QueryPerformanceCounter(&start);
> 	RunNonCollapsed();
> 	QueryPerformanceCounter(&stop);
> 	nonCollapsedTicks->QuadPart = stop.QuadPart - start.QuadPart;
> 	printf("NonCollapsed: %lu ticks\n", nonCollapsedTicks->QuadPart);
>
> 	printf("%d%%\n", (nonCollapsedTicks->QuadPart * 100) /
> collapsedTicks->QuadPart);
> }
>
> int main()
> {
> 	LARGE_INTEGER collapsedTicks;
> 	LARGE_INTEGER nonCollapsedTicks;
> 	LARGE_INTEGER totalCollapsedTicks;
> 	LARGE_INTEGER totalNonCollapsedTicks;
> 	int i;
>
> 	totalCollapsedTicks.QuadPart = 0;
> 	totalNonCollapsedTicks.QuadPart = 0;
> 	for (i = 0; i < 10; i++)
> 	{
> 		RunOnce(&collapsedTicks, &nonCollapsedTicks);
> 		totalCollapsedTicks.QuadPart += collapsedTicks.QuadPart;
> 		totalNonCollapsedTicks.QuadPart += nonCollapsedTicks.QuadPart;
> 	}
> 	printf("Average: %d%%\n", (totalNonCollapsedTicks.QuadPart * 100) /
> totalCollapsedTicks.QuadPart);
> }
>   
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> Ros-dev mailing list
> Ros-dev(a)reactos.com
> http://reactos.com:8080/mailman/listinfo/ros-dev
>

[Joseph Galbraith]

Gunnar Dalsnes wrote:
>   Phillip Susi wrote:
> >   Recently I was reading over some apache code and
> > they make extensive 
> >  use of macros for flow control.  I found myself becoming frustrated to 
> >  no end because I could not figure out what the hell was going on.  
> >  They looked like nice, neat function calls, but I could not for the 
> >  life of me figure out where the code was that was being called. 
>  
>  So if they had been function calls and not macros it would have been 
>  easier??? Or should we stop using functions also? ;-P 
Well, yes...

You can't step into macros in most debuggers (that I use.)

As Alex mentioned, it is a pain to try and sprinkle
debugging statements in a macro (if only because the
syntax is so ugly.)

Macros have ill defined side effects.  This is especially
damaging to code maintainability when macros look like
functions.

Macros are bug prone because they have additional unique
issues over normal code, they are hard to read, and hard
to debug.

C++ (and some latter version of C if I'm not mistaken)
added inline functions for a reason: so programmers would
stop using macros.

>   If you cant find a macro (simple text search) you
> probably wouldnt have 
>  understod the code _without_ the macros either;-P 
If I'm searching for something that looks like a function
call (especially if I'm searching for the definition not
the declaration), I might be looking in the wrong places
or searching using syntax that doesn't catch a #define.

Thanks,

Joseph

[Nathan Woods]

Gunnar Dalsnes wrote:

>   In win32k i use macros for flow control in the NtUser
> syscalls 
>  (functionally like a try/finally block) where its very important that 
>  some cleanup (release a lock) is always done:
>
>  BOOL NtFunc()
>  {
>    DECLARE_RETURN(BOOL);
>
>    Lock();
>    if (Stuff) RETURN(FALSE);
>    ....
>    RETURN(TRUE);
>
>  CLEANUP:
>    Unlock(Stuff);
>    DPRINT1("NtFunc returned %i\n", _ret_);
>    END_CLEANUP;
>  } 
Patterns like this can be done without macros.  I've always been partial 
to the following style:

BOOL NtFunc()
{
  BOOL bResult;
  void *pPointer = NULL;

  Lock();

  if (Stuff)
  {
    bResult = FALSE;
    goto cleanup;
  }
  ....

  bResult = TRUE;

cleanup:
  if (pPointer)
    free(pPointer);
  Unlock(stuff);
  DPRINT1("NtFunc returned %i\n", bResult);
  return bResult;
}

I know that many find goto's to be a crime against humanity, but I've 
always favored them for this sort of pattern.

-N

[Phillip Susi]

Gunnar Dalsnes wrote:

>  
>  In win32k i use macros for flow control in the NtUser syscalls 
>  (functionally like a try/finally block) where its very important that 
>  some cleanup (release a lock) is always done:
>
>  BOOL NtFunc()
>  {
>    DECLARE_RETURN(BOOL);
>
>    Lock();
>    if (Stuff) RETURN(FALSE);
>    ....
>    RETURN(TRUE);
>
>  CLEANUP:
>    Unlock(Stuff);
>    DPRINT1("NtFunc returned %i\n", _ret_);
>    END_CLEANUP;
>  }
>  
This is EXACTLY my point.  I intended to make macros much like that to 
handle error recovery and cleanup.  The problem is that it is very hard 
to understand.  It is much more clear to have something like this:

NTSTATUS NtFunc()
{
  Status = DoSomething();
  if( !NT_SUCCESS( Status ) )
    goto ret;
  Status = DoSomething2();
  if( !NT_SUCCESS( Status ) )
    goto cleanup1;
  Status = DoSomething3();
  if( !NT_SUCCESS( Status ) )
    goto cleanup2;
 
... etc

cleanup2:
  Cleanup2();
cleanup1:
  Cleanup1();
ret:
return Status;
}

At first it is temping to use macros to get rid of the duplicate if( 
!NTSUCCESS... lines, but anyone who knows C can read that code.  With 
the macros, they must first figure out how the macros work, which is 
often not very simple.  The most important attribute code can have is 
maintainability, and readability and simplicity lead to 
maintainability.  I finally realized that the real reason I wanted to 
make macros was because I was sick of retyping the same line of code 
over and over, but the readability of the code is far more important 
than saving the writer a few keystrokes. 

Your example code is one of the reasons that C++ developed exceptions.  
People kept writing code that was death by macros exactly that way for 
exactly that reason, so the developers of C++ decided it was a good idea 
to put support directly into the language for the needed functionality, 
without the use of macros. 

>  
>  If we should only invent stuff that ppl _already_ understand we just 
>  had to stop inventing.
>  
Inventing for the sake of inventing is a bad thing.  We invent things 
because they are useful.  When you invent things that change the 
language to the point that another person who is fluent in the language 
can not understand what you are saying, that is not useful. 

> >  
> >  Recently I was reading over some apache code and they make extensive 
> >  use of macros for flow control.  I found myself becoming frustrated 
> >  to no end because I could not figure out what the hell was going on.  
> >  They looked like nice, neat function calls, but I could not for the 
> >  life of me figure out where the code was that was being called. 
>
>  So if they had been function calls and not macros it would have been 
>  easier??? Or should we stop using functions also? ;-P 
Yes, because with a function call it is immediately apparent to anyone 
who knows C where the flow of code goes.  I spent some time trying to 
understand the flow of code there and finally gave up.  The macros were 
so complex that I couldn't figure them out in a reasonable amount of 
time, so I could not follow the flow of the code, and hence, was unable 
to maintain that code. 

>  
>  If you cant find a macro (simple text search) you probably wouldnt 
>  have understod the code _without_ the macros either;-P 
It isn't a matter of finding the macro.  Taking valuable time out from 
reading already complex code to go find a complex macro, then try to 
parse it and figure out what the real code you were looking at gets 
expanded to for the compiler, then figre out what the expanded code 
actually does can get very complex very quickly.  Remember, the number 
of bugs grows exponentially with the complexity of the system, so 
anything one can do to reduce complexity is a very good thing. 

>  
>  But off course you have to learn how the macros work to understand and 
>  utilize them, but that goes for any code. I cant see how learning a 
>  function call with 10+ params (typical nt syscall func) should be any 
>  easier than learning a simple macro. Thats just being lazy.
>
>  G.
>
>  
It is easier because when you see a function call you immediately know 
exactly where the code flows to.  It flows to the function that has that 
name, with the parameters listed, and then back again.  No more, and no 
less.  You don't have to figure out if it ends up passing control to 
three other places you don't know about, and if control eventually 
returns to this point or not.  You can easily find the called function 
which should have a comment block at the start explaining what the 
parameters mean in plain english.  Usually no further reading is 
required.  With a macro you have to find the macro, parse the terse SOB, 
go back to the code you were trying to understand in the first place, 
figure out how the compiler actually sees that code after the macro gets 
ahold of it, and then figure out what THAT code does.  It adds a lot of 
work with no real benefit.

[Alex Ionescu]

Gunnar Dalsnes wrote:

>  
>  If you come across a crash during an enum you just temporarily change 
>  the code to hunt the bug, just like normal bug hunting. Some of the 
>  list enums i changed were on this form... 
No, not like during normal bug hunting. Durnig normal bug hunting I add 
some dprints. During macro bug-hunting I revert the code to make it stop 
using the macro (potentially making a mistake and introducing a bug), 
add the debug prints, get it working, then attempt to add the macro 
again (and potentially make a mistake). More work for me, and more 
potential for bugs.

>  
>  for (HeadList = &Foo.Flink; Entry != HeadList; Entry = Entry->Flink)
>  {
>     Object = CONTAINING_RECORD(Entry, OBJECT, Link);
>     Object->stuff
>  }
>
>  ...and if you had to "debug" it like you showed youd had to edit the 
>  code just as well.  
That's because that loop is really ugly. I don't think I'd ever write a 
loop like that for manipulating list entries.

>   But for normal not-in-process-of-being-debugged-code i
> think the above 
>  code sux (even without the dprints). 
Yeah it does, it should be expanded to properly show each step line by 
line. Not macroized into a single unmaintainable ..thing.


Best regards,
Alex Ionescu

[Gunnar Dalsnes]

>   
>  Yeah it does, it should be expanded to properly show each step line by 
>  line.  
Thats your opinion. Seeing the boring internals of walking a list a 
billion times only pisses me off. I rather hide it where it cant be seen 
and focus on the usefull code instead.

>   Not macroized into a single unmaintainable ..thing.
There really is no way of "maintaining" how to walk a list. This is 
pretty much established "science". That why a macro is nice, because i 
dont want you to see it nor maintain it (or comment on the basic 
operations of list walking);-P

G.

[Thomas Weidenmueller]

Gunnar Dalsnes wrote:
> >  
> >  Yeah it does, it should be expanded to properly show each step line by
> >  line.  
>  
>  
>  Thats your opinion. Seeing the boring internals of walking a list a
>  billion times only pisses me off. I rather hide it where it cant be seen
>  and focus on the usefull code instead. 
Well, especially because it's the kernel, I agree with Alex. It shouln't
be hidden. I strongly prefer it the way it used to be.

> >   Not macroized
> > into a single unmaintainable ..thing. 
>  
>  
>  There really is no way of "maintaining" how to walk a list. This is
>  pretty much established "science". That why a macro is nice, because i
>  dont want you to see it nor maintain it (or comment on the basic
>  operations of list walking);-P 
Sometimes you want a loop to be slightly different. That's where the
macro fails. However, f.ex. inserting a list item or removing it is
_always_ the same, that's why there are macros for these and MS would've
defined such a macro if it was such a nice idea.

I can only reiterate, I'm never going to use these macros. At least not
in ntoskrnl or any drivers.

- Thomas

[Gregor Anich]

On Tuesday 27 September 2005 01:14, Alex Ionescu wrote:
>   gdalsnes(a)svn.reactos.com wrote:
> >  -reorder InsertXscendingOrder macro argument order
> > and update uses
> > -start to macrofy list enums in ntoskrnl using LIST_FOR_EACH macros
> > -use IsListEmpty some places instead of manual coding
> > -profile.c:KeStartProfile() zero correct buffer in (and not the NULL
> >  ptr;-) -improve LIST_FOR_EACH macros so that the iterator is set to NULL
> >  at end of enum -kthread.c:KiServiceCheck() set the shadowtable before
> >  calling the win32k proc/thread init funcs -apc.c:KiInsertQueueApc()
> >  insert special apcs in correct fifo order -apc.c:KeFlushQueueApc()
> >  simplify
> > -timer.c:KiExpireTimers() calling RemoveEntryList on a possibly empty list
> >  is not ok
> >
> >
> > Updated files:
> > trunk/reactos/include/reactos/helper.h
> > trunk/reactos/ntoskrnl/io/device.c
> > trunk/reactos/ntoskrnl/io/driver.c
> > trunk/reactos/ntoskrnl/io/file.c
> > trunk/reactos/ntoskrnl/io/fs.c
> > trunk/reactos/ntoskrnl/io/irp.c
> > trunk/reactos/ntoskrnl/io/pnpnotify.c
> > trunk/reactos/ntoskrnl/io/pnproot.c
> > trunk/reactos/ntoskrnl/ke/apc.c
> > trunk/reactos/ntoskrnl/ke/bug.c
> > trunk/reactos/ntoskrnl/ke/kqueue.c
> > trunk/reactos/ntoskrnl/ke/kthread.c
> > trunk/reactos/ntoskrnl/ke/profile.c
> > trunk/reactos/ntoskrnl/ke/queue.c
> > trunk/reactos/ntoskrnl/ke/timer.c
> > trunk/reactos/subsys/system/usetup/partlist.c
> > trunk/reactos/subsys/win32k/ntuser/class.c
> >
> > __________________________ 
>  Hi,
>
>  I'm going to have to voice my extreme outcry against this patch. It is
>  replacing clear and easily debuggable code by non-standard macros which
>  are impossible to debug. It is also going to create problems when people
>  which are not used to them (such as myself):
>
>  1) Have no idea what some LIST_FOR_EACH_SAFE thing is doing vs
>  LIST_FOR_EACH vs InsertListLifoSafeMaybeLoopOrWhoKnowsWhat
>  2) Will never code using these macros, and thus the source will become
>  full of both methods, which is totally ugly.
>
>  Sometimes, especially in kernel code, it's much better to have an
>  expanded logic then to use a macro. I wrote a great majority of the code
>  that has just been, imho, pollutted with these macros, and I don't
>  appreciate for someone to barge in and change my code for some macro
>  that I don't understand, nor wish to, because it only causes problems. I
>  take great care of the code I write and "own" it (in the programming
>  term) so that if a bug in it arises, I can quickly identify it and be
>  held responsible for it. The addition of these macros hinders that effort.
>
>  I don't mind someone using them inside his or her own code. I just mind
>  their usage in ntoskrnl, and even more so when it overwrites the clean,
>  clear code that I've written and am used to debugging.
>
>  Best regards,
>  Alex Ionescu 

You know, it also sucked when you changed the syntax of all the assembly files 
in ntoskrnl, thats when i stopped working on it. So, are you the only one who 
is allowed to do such changes because you believe to be the best?