Re: [ros-dev] [ros-diffs] [jimtabor] 30807: Revert 30780 for gdibatch. Size, space and time are very critical in here. If you dont know what your are doing? Do not play in here.

[Aleksey Bragin]

How come size, space and time matter in this particular case of where  
the variable is declared (it's still defined in the stack, wherever  
you write it)?

In the kernel, there is a rule to allocate all variables at top of  
the function (not in subsequent {} blocks). It should be extended to  
win32k too, it's cleaner, gives the developer an overview of all vars  
used in the function, and prevents possible cases of defining vars  
with the same name both atop of the function and in some subsequent  
block of code.

Not to mention this is not fully correct to do in a C language.

With the best regards,
Aleksey Bragin.

On Nov 27, 2007, at 4:14 AM, [e-mail] wrote:
> Author: jimtabor
> Date: Tue Nov 27 04:14:38 2007
> New Revision: 30807
>
> URL: http://svn.reactos.org/svn/reactos?rev=30807&view=rev
> Log:
> Revert 30780 for gdibatch. Size, space and time are very critical  
> in here. If you dont know what your are doing? Do not play in here.
>
> Modified:
>     trunk/reactos/subsystems/win32/win32k/objects/gdibatch.c
>
> Modified: trunk/reactos/subsystems/win32/win32k/objects/gdibatch.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/subsystems/ 
> win32/win32k/objects/gdibatch.c?rev=30807&r1=30806&r2=30807&view=diff
> ====================================================================== 
> ========
> --- trunk/reactos/subsystems/win32/win32k/objects/gdibatch.c  
> (original)
> +++ trunk/reactos/subsystems/win32/win32k/objects/gdibatch.c Tue  
> Nov 27 04:14:38 2007
> @@ -20,7 +20,6 @@
>  {
>    PDC dc = NULL;
>    PDC_ATTR Dc_Attr = NULL;
> PGDIBSSETBRHORG pgSBO;
> if (hDC)
> {
>   dc = DC_LockDc(hDC);
>
> @@ -40,6 +39,7 @@
>          break;
>       case GdiBCSetBrushOrg:
>       {
>    PGDIBSSETBRHORG pgSBO;
>    if (!dc) break;
>    pgSBO = (PGDIBSSETBRHORG) pHdr;
>    Dc_Attr->ptlBrushOrigin = pgSBO->ptlBrushOrigin;

[Thomas Weidenmueller]

This change and claim is misleading. Actually it makes absolutely no
difference as the compiler most likely does this automatically anyway.
It actually most likely allocates the variable at the beginning of the
function, too. This change has zero effect on the code that is being
output. It's absolutely pointless making assuptions about code size or
speed depending on how you declare local variables.

- Thomas

Aleksey Bragin wrote:
> How come size, space and time matter in this particular case of where  
> the variable is declared (it's still defined in the stack, wherever  
> you write it)?
>
> In the kernel, there is a rule to allocate all variables at top of  
> the function (not in subsequent {} blocks). It should be extended to  
> win32k too, it's cleaner, gives the developer an overview of all vars  
> used in the function, and prevents possible cases of defining vars  
> with the same name both atop of the function and in some subsequent  
> block of code.
>
> Not to mention this is not fully correct to do in a C language.
>
> With the best regards,
> Aleksey Bragin.
>
> On Nov 27, 2007, at 4:14 AM, [e-mail] wrote:
> > Author: jimtabor
> > Date: Tue Nov 27 04:14:38 2007
> > New Revision: 30807
> >
> > URL: http://svn.reactos.org/svn/reactos?rev=30807&view=rev
> > Log:
> > Revert 30780 for gdibatch. Size, space and time are very critical  
> > in here. If you dont know what your are doing? Do not play in here.
> >
> > Modified:
> >     trunk/reactos/subsystems/win32/win32k/objects/gdibatch.c
> >
> > Modified: trunk/reactos/subsystems/win32/win32k/objects/gdibatch.c
> > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/subsystems/ 
> > win32/win32k/objects/gdibatch.c?rev=30807&r1=30806&r2=30807&view=diff
> > ====================================================================== 
> > ========
> > --- trunk/reactos/subsystems/win32/win32k/objects/gdibatch.c  
> > (original)
> > +++ trunk/reactos/subsystems/win32/win32k/objects/gdibatch.c Tue  
> > Nov 27 04:14:38 2007
> > @@ -20,7 +20,6 @@
> >  {
> >    PDC dc = NULL;
> >    PDC_ATTR Dc_Attr = NULL;
> > PGDIBSSETBRHORG pgSBO;
> > if (hDC)
> > {
> >   dc = DC_LockDc(hDC);
> >
> > @@ -40,6 +39,7 @@
> >          break;
> >       case GdiBCSetBrushOrg:
> >       {
> >    PGDIBSSETBRHORG pgSBO;
> >    if (!dc) break;
> >    pgSBO = (PGDIBSSETBRHORG) pHdr;
> >    Dc_Attr->ptlBrushOrigin = pgSBO->ptlBrushOrigin;
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev