Bug 160137 - CharacterColor.h != and == implementation causing alignment trap on ARM (and probably other) processors (x86 not affected)
Summary: CharacterColor.h != and == implementation causing alignment trap on ARM (and ...
Status: RESOLVED FIXED
Alias: None
Product: konsole
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Konsole Developer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-03-31 00:26 UTC by Alessandro Briosi
Modified: 2008-03-31 00:58 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alessandro Briosi 2008-03-31 00:26:18 UTC
Version:           2.0 up (using Devel)
Installed from:    Compiled sources
Compiler:          gcc-4.1.2 Porting konsole4 to Qtopia made me find this bug, which I think should be fixed.
OS:                Linux

The current implementation of the inline bool operators != and ==
causes an alignment trap on arm cpu. 
It's not memory alignment safe.
I found this porting konsole 2 to Qtopia.

Current implementation:
---------------------------------------
inline bool operator == (const CharacterColor& a, const CharacterColor& b)
{
  return *reinterpret_cast<const quint32*>(&a._colorSpace) ==
         *reinterpret_cast<const quint32*>(&b._colorSpace);
}

inline bool operator != (const CharacterColor& a, const CharacterColor& b)
{
  return *reinterpret_cast<const quint32*>(&a._colorSpace) !=
         *reinterpret_cast<const quint32*>(&b._colorSpace); 
}

proposed new implementation fixing the problem and memory alignemnt safe:
-------------------------------------------------
inline bool operator == (const CharacterColor& a, const CharacterColor& b)
{
  return (a._colorSpace == b._colorSpace && a._u == b._u
                          && a._v == b._v && a._w == b._w);
}

inline bool operator != (const CharacterColor& a, const CharacterColor& b)
{
  return (a._colorSpace != b._colorSpace || a._u != b._u
                          || a._v != b._v || a._w != b._w);
}
Comment 1 Alessandro Briosi 2008-03-31 00:28:39 UTC
Forgot to mention the file:
File is CharacterColor.h
Comment 2 Robert Knight 2008-03-31 00:35:09 UTC
Hi,

I didn't write that code myself but it seems to me that it is unnecessary to have them at all.  Alignment problem aside, the operator==() and operator!=() methods are equivalent to the built-in defaults which the compiler generates.

Comment 3 Robert Knight 2008-03-31 00:46:43 UTC
> Alignment problem aside, the operator==() and operator!=() methods
> are equivalent to the built-in defaults which the compiler generates. 

Sorry, moment of stupidity, I was thinking of operator=().  An alterative would be to use memcmp() in operator==() and then define operator!=() as !operator==(). 

Comment 4 Robert Knight 2008-03-31 00:58:09 UTC
SVN commit 792018 by knight:

Re-write CharacterColor::operator==(), CharacterColor::operator!=().
Previous implementation caused memory alignment trap on ARM.

BUG:160137


 M  +2 -5      CharacterColor.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=792018
Comment 5 Robert Knight 2008-03-31 00:58:18 UTC
SVN commit 792019 by knight:

Compare CharacterColor members explicitly rather than using memcpy for clarity.

CCBUG: 160137


 M  +4 -1      CharacterColor.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=792019