This is the mail archive of the ecos-patches@sourceware.org mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Speed-up sprintf() family of functions.


Jonathan Larmour <jifl@eCosCentric.com> writes:

> Sergei Organov wrote:
>> Andrew Lunn <andrew@lunn.ch> writes:
>>
>>>>The updated patch is attached.
>>>>
>>>
>>>Hi Sergei
>>>
>>>Im looking at the patch now ready for including into anoncvs. I have
>>>one minor problem which needs fixing. When i compile for the linux
>>>target with gcc 4.1.2 i get a warning:
>>
>>
>> There doesn't seem to be a way to actually fix this warning, only some
>> hacks to shut-up GCC. What hack in particular do you use to prevent this
>> kind of warnings in other places of eCos where it occurs?
>
> There is a correct solution. Create a one-off union type, containing a
> FILE and a Cyg_VsnprintfStream and assign the stream to the
> Cyg_VsnprintfStream, and then take the address of the FILE
> member. Alignment constraints are dealt with by the fact that streams
> are created with the correct alignment - but GCC doesn't know that.

Well, I can definitely do that, but I don't think it's "correct"
solution, -- it's one kind of ugly hacks that will make GCC
shut-up. Alignment has nothing to do about it, I believe, -- gcc warns
about possible aliasing rules violation, not alignment.

Please note that aliasing rules violation may occur only when a pointer
is *dereferenced*, never when a pointer is converted from one type to
another, but GCC is not currently clever enough to detect *actual*
aliasing violations, so it warns about pointers conversions instead that
does produce many false-positives. GCC folks say that this warning
actually means "go fix your *interfaces*", and that is far beyond the
scope of my patch.

I've just reproduced this warning with gcc-4.1.1. You know what? Just
adding an unused field of type "char" to my Cyg_VsnprintfStream class
makes the warning go away. BTW, this is why it doesn't warn without my
patch, -- because Cyg_StdioStream does contain
"cyg_uint8 readbuf_char;".

Is it "correct" fix to add otherwise unused char field to my class?

Please note that if I compile with -Wstrict-aliasing=2, GCC gives
warning in vsnprintf.cxx (in the old similar code) even in unpatched
eCos.

Another way to "fix" it is:

-    return vfnprintf( (FILE *)&stream, size, format, arg );
+    return vfnprintf( (FILE *)(void*)&stream, size, format, arg );

is it "correct" fix?

Yet another "fix" is to change the FILE typedef in stdio.h to be like
this (though this won't prevent warnings from -Wstrict-aliasing=2):

typedef struct {
  CYG_ADDRESS addr;  // For correct alignment
  char c[9999];      // For forcing aliasing of anything
} FILE;


Overall, I insist that this warning is bogus, and therefore there is no
"correct" fix for it. If we need to just shut-up compiler I'd prefer
the (void*) hack above.

What do you think?

-- Sergei Organov.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]