Universal escaping in KConfigINI: Summary and patch

[Andreas Hartmetz]

On Thursday 24 May 2007 20:37:47 Simon Hausmann wrote:
> 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

Heh, this is great. Exactly what I thought would happen if we provided such a 
functions in kdelibs happened, except that the functions were already there 
in Qt instead of kdelibs.
In this particular case, I'd let the NIH syndrome win in KConfigIni (faster 
than constructing QByteArrays) and otherwise hope that people will find the 
functions that are there in Qt.