Universal escaping in KConfigINI: Summary and patch

[Simon Hausmann]

On Thursday 24 May 2007 20:24:45 Andreas Hartmetz wrote:
> On Thursday 24 May 2007 20:14:47 Ingo Klöcker wrote:
> > On Thursday 24 May 2007 19:05, Oswald Buddenhagen wrote:
> > > On Thu, May 24, 2007 at 06:47:41PM +0200, Andreas Hartmetz wrote:
> > > > The switch blocks are not inelegant, they are really ugly :)
> > > > The use of switch statements was a conscious decision.
> > > > These implementations have the advantage of obviously not having
> > > > any index out of bounds errors, and I trust the compiler to make a
> > > > table out of switch statements where possible. The simplistic
> > > > switch statement should be easy to understand by the compiler so it
> > > > can optimize well.
> > > > If you are reasonably sure that one of the target compilers won't
> > > > do this, I'll change it.
> > >
> > > no sane compiler will automatically generate an almost-256-entry
> > > table for 16 values. ;) if you used the *low* nibble in toHex it
> > > *might* optimize it, but given that the manual solution is much nicer
> > > i would not take chances.
> > > i see little chance for toChar being optimized, too, but you can look
> > > at the -S output ... but even if, it would be still ugly for almost
> > > no benefit (the bounds checking and indexing will outweight the few
> > > comparisons and arithmetics easily).
> >
> > Shouldn't there be a single, fast implementation of those two
> > conversions somewhere in kdelibs/kdecore? I bet those conversions have
> > been re-implemented all over KDE.
> >
> > Please don't add another private version of those methods but instead
> > add public versions for example to
> > trunk/KDE/kdelibs/kdecore/text/kstringhandler.*
>
> Ah, I thought the same, but then I thought that nobody will find these
> functions anyway. Doing the Right Thing would still make sense...
> My current versions of these functions look like this now, for the record:
>
> inline static char hexToChar(const char *str)
> {
>     char ret = 0;
>     char c;
>     for (int i = 0; i < 2; i++) {
>         ret <<= 4;
>         c = str[i];
>
>         if (c >= '0' && c <= '9') {
>             ret |= c - '0';
>         } else if (c >= 'a' && c <= 'f') {
>             ret |= c - 'a' + 0x0a;
>         } else if (c >= 'A' && c <= 'F') {
>             ret |= c - 'A' + 0x0a;
>         } else {
>              //TODO: warning
>             return 'x';
>         }
>     }
>     return ret;
> }
>
> inline static void charToHex(char c, char *str)
> {
>     static const char lookup[] = {
>         '0', '1', '2', '3', '4', '5', '6', '7',
>         '8', '9', 'a', 'b', 'c', 'd', 'e', 'f'
>     };
>
>     for (int i = 0; i < 2; i++) {
>         str[i] = lookup[c >> 4];
>         c <<= 4;
>     }
> }

Why not simply use QByteArray::fromHex and QByteArray::toHex? :)

Simon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20070524/721a9140/attachment.sig>