Re: [PATCH] Replace loop by memset in sal/typesconfig/typesconfig.c

Quick question: is this code security-sensitive at all?

I know that there has been some security issues related to one compiler
(Microsoft's) that was optimizing away the memset in some cases. So the
suggested workaround was a loop. Reference: Writing Secure Code.

The other question is: Are we still supporting building with such a
compiler? My bad memory tells me that VS 2008 and later should not have
this behavior anymore, but it is worthwhile to double-check.

On Tuesday, January 8, 2013, Stephan Bergmann wrote:

> On 12/24/2012 06:44 PM, Julien Nabet wrote:
>
> > On 24/12/2012 18:29, Norbert Thiebaud wrote:
> >
> > > On Mon, Dec 24, 2012 at 8:45 AM, julien2412<serval2412@yahoo.fr>  wrote:
> > >
> > > > By taking a look at the file  sal/typesconfig/typesconfig.c,
> > > > GetAlignment
> > > > function, I wonder if we could replace the for loop by a memset to
> > > > optimize
> > > > a bit.
> > > > So here's a straightforward patch:
> > > > diff --git a/sal/typesconfig/typesconfig.**c
> > > > b/sal/typesconfig/typesconfig.**c
> > > > index 473f07a..ef52c5f 100644
> > > > --- a/sal/typesconfig/typesconfig.**c
> > > > +++ b/sal/typesconfig/typesconfig.**c
> > > > @@ -262,11 +262,9 @@ int GetAlignment( Type eT )
> > > >   {
> > > >     char  a[ 16*8 ];
> > > >     long  p = (long)(void*)a;
> > > > -  int   i;
> > > >
> > > >     /* clear a[...] to set legal value for double access */
> > > > -  for ( i = 0; i<  16*8; i++ )
> > > > -    a[i] = 0;
> > > > +  memset(a, 0, sizeof(a));
> > > >
> > > >     p = ( p + 0xF )&  ~0xF;
> > > >     for ( i = 1; i<  16; i++ )
> > > >
> > > > Would it be ok?
> > > well you can't remove
> > >   int i;
> > >
> > > since it is still used in the second for loop.
> > >
> > > and I doubt that that code is run very often... namely 4 times per
> > > execution of the typesconfig executable... which in turn run dozens of
> > > fprintf...
> > > I'd say that this micro-optimization would be completely
> > > un-measurable, and may or may not be an optimization at all depending
> > > on how the memset is treated.
> > > but sure, it is 'ok', as in it won't hurt anything.
> The benefit of memset over an explicit loop here would be shorter, more
> obvious (IMO) code, so I'd suggest you do the change after all.
>
> Stephan
>
>  ok let's forget it, I suppose that the one who made this loop has
> > certainly good reasons for this + you must be right, micro-optimization
> ______________________________**_________________
> LibreOffice mailing list
> LibreOffice@lists.freedesktop.org
> http://lists.freedesktop.org/**mailman/listinfo/libreoffice<http://lists.freedesktop.org/mailman/listinfo/libreoffice>


-- 
Marc-André LAVERDIÈRE
"Perseverance must finish its work so that you may be mature and complete,
not lacking anything." -James 1:4
http://asimplediscipleslife.blogspot.com/
mlaverd.theunixplace.com