This is the mail archive of the
ecos-patches@sourceware.org
mailing list for the eCos project.
Re: [PATCH] Speed-up sprintf() family of functions.
Sergei Organov wrote:
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.
Strictly it's the correct solution. I was involved in some of the
discussion when this was first implemented in GCC. A union is the official
and permanently portable way to solve it. Yes it's somewhat pointless
syntactic sugar, but ......
Alignment has nothing to do about it, I believe, -- gcc warns
about possible aliasing rules violation, not alignment.
Indeed, but separate from aliasing specifically, casting between two types
can cause problems if one of the types has stronger alignment constraints.
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.
We don't want to reveal what's behind a FILE, so indeed, no.
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?
Sounds wasteful.
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?
"Correct", no. But I take your point about not actually dereferencing the
pointer here, so aliasing is never, in practice, going to be an issue. So
let's just go with this one, since it's simple. I'm about to go offline
for an extended period, so it can wait for me to return, or perhaps Andrew
can check it in with just that change.
Jifl
--
eCosCentric http://www.eCosCentric.com/ The eCos and RedBoot experts
Company legal info, address and number: http://www.ecoscentric.com/legal
------["The best things in life aren't things."]------ Opinions==mine