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

[Marc-André Laverdière]

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 at 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 at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/libreoffice/attachments/20130114/e78f9c03/attachment.html>