Re: [ros-cvs] CVS Update: reactos

[Aliberti Emanuele]

chorns(a)cvs.reactos.com wrote:

>  CVSROOT:	/CVS/ReactOS
> Module name:	reactos
> Repository:	reactos/tools/
> Changes by:	chorns(a)mok.osexperts.com	04/10/28 12:02:01
>
> Log message:
> 	2004-10-28  Casper S. Hornstrup  <chorns(a)users.sourceforge.net>
> 	
> 	* drivers/net/tcpip/tests/setup.c: Include windows.h.
> 	* lib/gdiplus/tests/setup.c: Ditto.
> 	* lib/kernel32/tests/setup.c: Ditto.
> 	* lib/ws2_32/tests/setup.c: Ditto.
> 	* subsys/win32k/tests/setup.c: Ditto.
> 	* ntoskrnl/tests/setup.c: Ditto.
> 	* ntoskrnl/include/ntoskrnl.h: Include test.h.
> 	* ntoskrnl/mm/virtual.c (NtLockVirtualMemoryInternal): New function.
> 	(NtLockVirtualMemory): Use NtLockVirtualMemoryInternal.
>   
>  
Should we define a sort of "best practice" in inserting a little
"p" 
after the subsystem prefix and before the proper function's o 
structure's name, to mark the *p*rivate symbols, or use the "Internal" 
suffix?

1) NtpLockVirtualMemory (MmpLockVirtualMemory)
2) NtLockVirtualMemoryInternal

IMHO, not only an aesthetics problem.

Option (1) means that any symbol in ntoskrnl/include have a "p" or "P"

in their names.

>  	* ntoskrnl/tests/Makefile (TARGET_CFLAGS): Add
> -I../include.
> 	* ntoskrnl/tests/stubs.tst: Remove stubs for functions in libgcc.
> 	* regtests/shared/regtests.h: Make independent of windows.h.
> 	* tools/helper.mk (run): Link with libgcc.
> 	* tools/regtests.c (EXESTUB): Include windows.h.
> 	* ntoskrnl/include/internal/test.h: New file.
> 	* ntoskrnl/tests/tests/VirtualMemory.c: Ditto.
>
>   
>  
Emanuele

[Alex Ionescu]

Aliberti Emanuele wrote:

>   chorns(a)cvs.reactos.com wrote:
>
> >   CVSROOT:    /CVS/ReactOS
> >  Module name:    reactos
> >  Repository:    reactos/tools/
> >  Changes by:    chorns(a)mok.osexperts.com    04/10/28 12:02:01
> >
> >  Log message:
> >      2004-10-28  Casper S. Hornstrup  <chorns(a)users.sourceforge.net>
> >      
> >      * drivers/net/tcpip/tests/setup.c: Include windows.h.
> >      * lib/gdiplus/tests/setup.c: Ditto.
> >      * lib/kernel32/tests/setup.c: Ditto.
> >      * lib/ws2_32/tests/setup.c: Ditto.
> >      * subsys/win32k/tests/setup.c: Ditto.
> >      * ntoskrnl/tests/setup.c: Ditto.
> >      * ntoskrnl/include/ntoskrnl.h: Include test.h.
> >      * ntoskrnl/mm/virtual.c (NtLockVirtualMemoryInternal): New function.
> >      (NtLockVirtualMemory): Use NtLockVirtualMemoryInternal.
> >   
> >  
>  Should we define a sort of "best practice" in inserting a little
> "p" 
>  after the subsystem prefix and before the proper function's o 
>  structure's name, to mark the *p*rivate symbols, or use the "Internal" 
>  suffix?
>
>  1) NtpLockVirtualMemory (MmpLockVirtualMemory)
>  2) NtLockVirtualMemoryInternal
>
>  IMHO, not only an aesthetics problem.
>
>  Option (1) means that any symbol in ntoskrnl/include have a "p" or
> "P" 
>  in their names.
>
> >       * ntoskrnl/tests/Makefile (TARGET_CFLAGS):
> > Add -I../include.
> >      * ntoskrnl/tests/stubs.tst: Remove stubs for functions in libgcc.
> >      * regtests/shared/regtests.h: Make independent of windows.h.
> >      * tools/helper.mk (run): Link with libgcc.
> >      * tools/regtests.c (EXESTUB): Include windows.h.
> >      * ntoskrnl/include/internal/test.h: New file.
> >      * ntoskrnl/tests/tests/VirtualMemory.c: Ditto.
> >
> >   
> >  
>  Emanuele
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.com
>  http://reactos.com:8080/mailman/listinfo/ros-dev
>  
Thanks for bringing this up Emanuele!

I am strongly for using "p" for private, since that's what I've always 
used, and that's also what MS uses. It's also a bit shorter.
In case anyone is interested, here are the best practices from Microsoft:

Cc->None
Ex->Exp
Dbg->Dbgk
FsRtl->FsRtlp
Hal->Halp
Io->Iop
Ke->Ki
Lpc->Lpcp
Ldr->Ldrp
Mm->Mi
Nt->None
Ob->Obp
Rtl->Rtlp
Se->Sep
Vf->Vi
Wmi->Wmip
Zw->None


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. oktober 2004 00:47
>  To: ReactOS Development List
>  Subject: Re: [ros-dev] Re: [ros-cvs] CVS Update: reactos
>  
>  Aliberti Emanuele wrote:
>  
> >   chorns(a)cvs.reactos.com wrote:
> >
> > > CVSROOT:    /CVS/ReactOS
> > > Module name:    reactos
> > > Repository:    reactos/tools/
> > > Changes by:    chorns(a)mok.osexperts.com    04/10/28 12:02:01
> > >
> > > Log message:
> > >     2004-10-28  Casper S. Hornstrup  <chorns(a)users.sourceforge.net>
> > >     
> > >     * drivers/net/tcpip/tests/setup.c: Include windows.h.
> > >     * lib/gdiplus/tests/setup.c: Ditto.
> > >     * lib/kernel32/tests/setup.c: Ditto.
> > >     * lib/ws2_32/tests/setup.c: Ditto.
> > >     * subsys/win32k/tests/setup.c: Ditto.
> > >     * ntoskrnl/tests/setup.c: Ditto.
> > >     * ntoskrnl/include/ntoskrnl.h: Include test.h.
> > >     * ntoskrnl/mm/virtual.c (NtLockVirtualMemoryInternal):  
>  New function.
> > >      
> > > (NtLockVirtualMemory): Use NtLockVirtualMemoryInternal.
> > >   
> > >  
> >  Should we define a sort of "best practice" in inserting a  
>  little
> "p" 
> >   after the subsystem prefix and before the proper
> > function's o 
> >  structure's name, to mark the *p*rivate symbols, or use the  
> "Internal"
> >   suffix?
> >
> >  1) NtpLockVirtualMemory (MmpLockVirtualMemory)
> >  2) NtLockVirtualMemoryInternal
> >
> >  IMHO, not only an aesthetics problem.
> >
> >  Option (1) means that any symbol in ntoskrnl/include have a  
>  "p" or
> "P" 
> >   in their names.
> >
> > >     * ntoskrnl/tests/Makefile (TARGET_CFLAGS): Add -I../include.
> > >     * ntoskrnl/tests/stubs.tst: Remove stubs for functions  
>  in libgcc.
> > >       *
> > > regtests/shared/regtests.h: Make independent of windows.h.
> > >      * tools/helper.mk (run): Link with libgcc.
> > >      * tools/regtests.c (EXESTUB): Include windows.h.
> > >      * ntoskrnl/include/internal/test.h: New file.
> > >      * ntoskrnl/tests/tests/VirtualMemory.c: Ditto.
> > >
> > >   
> > >  
> >  Emanuele
> >  _______________________________________________
> >  Ros-dev mailing list
> >  Ros-dev(a)reactos.com
> >  http://reactos.com:8080/mailman/listinfo/ros-dev
> >  
>  Thanks for bringing this up Emanuele!
>  
>  I am strongly for using "p" for private, since that's what 
>  I've always used, and that's also what MS uses. It's also a 
>  bit shorter.
>  In case anyone is interested, here are the best practices 
>  from Microsoft:
>  
>  Cc->None
>  Ex->Exp
>  Dbg->Dbgk
>  FsRtl->FsRtlp
>  Hal->Halp
>  Io->Iop
>  Ke->Ki
>  Lpc->Lpcp
>  Ldr->Ldrp
>  Mm->Mi
>  Nt->None
>  Ob->Obp
>  Rtl->Rtlp
>  Se->Sep
>  Vf->Vi
>  Wmi->Wmip
>  Zw->None
>  
>  
>  Best regards,
>  Alex Ionescu 
I didn't do that so it wouldn't conflict with published private/internal
functions known from Windows where we can't change the signature. This
function is made to be testable and run in user-mode so it needs to have
a certain signature. I am open to suggestions and if there is no problem
in not having the exact signature Microsoft use then I'm all for using p
or i instead.

Casper

[Filip Navara]

Alex Ionescu wrote:
...

>   Dbg->Dbgk 

Actually "Dbgk" is for user mode debugging support and not a prefix for 
internal "Dbg" routines.

Regards,
Filip

[Alex Ionescu]

Filip Navara wrote:

>   Alex Ionescu wrote:
>  ...
>
> >   Dbg->Dbgk 
>
>
>  Actually "Dbgk" is for user mode debugging support and not a prefix 
>  for internal "Dbg" routines.
>
>  Regards,
>  Filip
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.com
>  http://reactos.com:8080/mailman/listinfo/ros-dev
>  
Hi Filip,

Thanks; I forgot a letter, that should've been:

Dbgk->Dbgkp

Best regards,
Alex Ionescu

"Aliberti Emanuele" <ea(a)iol.it> wrote:

>   Should we define a sort of "best practice"
> in inserting a little "p"
>  after the subsystem prefix and before the proper function's o
>  structure's name, to mark the *p*rivate symbols, or use the "Internal"
>  suffix?
>
>  1) NtpLockVirtualMemory (MmpLockVirtualMemory)
>  2) NtLockVirtualMemoryInternal 
I think the function in question should be called MiLockVirtualMemory
because it is an internal function of the memory manager. And the Nt- and
Zw-Prefixes are only used for funtions that are exposed via the Int2e
interface. Therefore, internal Nt- or Zw-Functions do not exist.

Regards,
Eric

[Alex Ionescu]

Eric Kohl wrote:

>  "Aliberti Emanuele" <ea(a)iol.it> wrote:
>
>   
>
> >  Should we define a sort of "best
> > practice" in inserting a little "p"
> > after the subsystem prefix and before the proper function's o
> > structure's name, to mark the *p*rivate symbols, or use the "Internal"
> > suffix?
> >
> > 1) NtpLockVirtualMemory (MmpLockVirtualMemory)
> > 2) NtLockVirtualMemoryInternal
> >     
> >  
> I think the function in question should be called MiLockVirtualMemory
> because it is an internal function of the memory manager. And the Nt- and
> Zw-Prefixes are only used for funtions that are exposed via the Int2e
> interface. Therefore, internal Nt- or Zw-Functions do not exist.
>
> Regards,
> Eric
>
> _______________________________________________
> Ros-dev mailing list
> Ros-dev(a)reactos.com
> http://reactos.com:8080/mailman/listinfo/ros-dev
>
>   
>  
Hi Eric,

Very good point, I hadn't even noticed this.
Yes, the correct name would be MiLockVirtualMemory.

Best regards,
Alex Ionescu