Re: [ros-cvs] CVS Update: reactos

[Aliberti Emanuele]

[e-mail] wrote:
> CVSROOT:	/CVS/ReactOS
> Module name:	reactos
> Repository:	reactos/tools/
> Changes by:	[e-mail]	04/10/28 12:02:01
>
> Log message:
>    2004-10-28  Casper S. Hornstrup  [e-mail]
> 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:
> [e-mail] wrote:
> > CVSROOT:    /CVS/ReactOS
> > Module name:    reactos
> > Repository:    reactos/tools/
> > Changes by:    [e-mail]    04/10/28 12:02:01
> >
> > Log message:
> >     2004-10-28  Casper S. Hornstrup  [e-mail]
> > * 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?
> NtpLockVirtualMemory (MmpLockVirtualMemory)
> 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
> [e-mail]
> 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: [e-mail] 
> [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:
> > [e-mail] wrote:
> > > CVSROOT:    /CVS/ReactOS
> > > Module name:    reactos
> > > Repository:    reactos/tools/
> > > Changes by:    [e-mail]    04/10/28 12:02:01
> > >
> > > Log message:
> > >     2004-10-28  Casper S. Hornstrup  [e-mail]
> > > * 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?
> > NtpLockVirtualMemory (MmpLockVirtualMemory)
> > 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
> > [e-mail]
> > 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
> [e-mail]
> 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" [e-mail] 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?
> NtpLockVirtualMemory (MmpLockVirtualMemory)
> 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" [e-mail] 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?
> > NtpLockVirtualMemory (MmpLockVirtualMemory)
> > 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
> [e-mail]
> 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