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.


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


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