[PATCH] Remove useless members of DiveItem
Thiago Macieira
thiago at macieira.org
Thu Apr 25 08:39:45 PDT 2013
On quinta-feira, 25 de abril de 2013 08.15.14, Dirk Hohndel wrote:
> > or dive(NULL), as I thought we had agreed to use NULL for pointers, not
> > zeroes. I had to look at the class member to see if it was a pointer or
> > not. Then I had to think for a second about what the "default
> > constructor" for a pointer does -- yes, it does a zero-initialisation,
> > but it leads to head- scratching.
>
> I totally agree. My excuse here is that I didn't even know what this
> code was supposed to be doing. I guess dive(NULL) makes this much
> clearer. Which of course also reminded me of the fact that we don't
> check for NULL when we dereference it later...
One of those things Coverity or other static code checkers catch for you.
> > > > + const QString diveDateTime() const { return
> > > > QString::fromUtf8(get_dive_date_string(dive->when)); }
> >
> > This is the diveDateTime() from the other thread. Does anyone know how
> > this
> > could return badly encoded data?
>
> It should return Utf8 at all times. And I thought you needed to tell
> QString which encoding is 'incoming' :-/
You do, by telling it the encoding of the incoming char* when you create the
QString. Hence my suggestion later on.
> > Given the solution that was proposed there, I would say that the problem
> > has been _fixed_. Can someone verify?
> >
> > Here's why: the proposed workaround was to do fromUtf8(string.toLatin1()),
> > which indicates that the string contained the opposite: UTF-8 data
> > interpreted via fromLatin1(). Since this function is now doing
> > fromUtf8(), it can't be wrong any more.
>
> So is this a complicated way of saying "if it's utf8 we don't need a
> conversion? I'm still not clear on how QString actually works...
No, this was a complicated way of saying "I think the bug is fixed". This
particular code path looks fine here, so I can't see how the bug would show up.
> > > > - const QString& diveLocation() const { return location; }
> > > > - const QString& diveSuit() const { return suit; }
> > > > + const QString diveLocation() const { return dive->location; }
> > > > + const QString diveSuit() const { return dive->suit; }
> > What's the encoding for the dive location? Are we missing
> > QString::fromUtf8() here?
>
> Oh, so we need this after all? Yes, location is user input (or
> downloaded from a web site, or - in rare cases like the Uemis Zurich -
> downloaded from a dive computer. It's expected to be Utf8.
>
> But now you have me even more confused then before...
>
> When exactly do I need the fromUtf8 thingy???
Every time your char* is UTF-8.
Let's see... is Linus reading this email? I hope not... anyway QString holds
UTF-16 data internally. So when you give it a char*, it needs to know how to
decode that into UTF-16.
We can probably do this somewhere:
QTextCodec::setCodecForCStrings(QTextCodec::codecForName("UTF-8"))
So any time you create a QString from char*, it will interpret that as UTF-8.
Conversely, whenever you create a char* from a QString, it will give you
UTF-8.
That is also the default behaviour in Qt 5. And it can't be changed there.
--
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
Software Architect - Intel Open Source Technology Center
PGP/GPG: 0x6EF45358; fingerprint:
E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.hohndel.org/pipermail/subsurface/attachments/20130425/173f82fb/attachment.sig>
More information about the subsurface
mailing list