[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