Re: [Development] API review for a new QDnsResolver class
On Thu, Nov 3, 2011 at 12:40 PM, Jeremy Lainé jeremy.la...@m4x.org wrote:
Based on some initial feedback I received regarding DNS SRV support in Qt,
I have refactored my proposed code and introduced a QDnsResolver class
which I would like to submit for API review. The point of this class is to
provide a QNAM-style asynchronous API to perform DNS lookups.
The resolver object itself looks like:
class Q_NETWORK_EXPORT QDnsResolver : public QObject
{
Q_OBJECT
public:
QDnsResolver(QObject *parent = 0);
~QDnsResolver();
QDnsReply *lookupService(const QString serviceName, const QString
domainName);
private:
Q_DECLARE_PRIVATE(QDnsResolver)
};
The more I think about it, the more I think it is important to fix this:
who is responsible for the lifetime of the QDnsReply object?
This API does not make that clear. I like the pattern in itself (also in
QNAM), but I do think it would be an improvement if we were to use a shared
pointer to the reply object. That at least makes clear who has ownership of
the object, and prevents memory leaks when people don't realize they are
supposed to delete the object.
Alternatively, perhaps a look at QFuture would help. QFuture is another way
results that are not yet ready are handled in Qt, but this time it is
returned as a value instead of as a pointer. It would be nice we could come
up with a single approach for these kinds of things and use that all over
the place...
Another concern is the question why you need a QDnsResolver object at all.
It only has a single method, and it does not seem to benefit from it being
a QObject at all at the moment, unless you plan to mimick the API of QNAM
and add signals and slots on the Resolver object itself. If not, then it
might as well just be a static method of the class, or just a method in a
namespace called QDnsResolver or something like that. Currently, the user
will have to instantiate the QDnsResolver object, and call the
lookupService method. It is not so clear what needs to happen to that
instance after that. Can it be savely deleted again while the request is
still running?
Andre'
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] API review for a new QDnsResolver class
On 11/03/2011 08:32 PM, ext Jeremy Lainé wrote: Le Nov 3, 2011 à 7:10 PM, Peter Hartmann a écrit : On 11/03/2011 04:12 PM, ext Thiago Macieira wrote: (...) The DNS query is not the problem. A query is always composed of a domain name being queried, along with the DNS class (Internet, no one uses anything else for real purposes) and the record type. For the record type, it's very easy to just list all types and also say that the user can use other types from the DNS documentation. See the ns_type enum in /usr/include/arpa/nameser.h [1]. Yes, the query is quite simple, that is why I was wondering whether we need a class for it at all. Probably not. In the end I ended just storing the request type and encoded query domain in QDnsReplyPrivate. I have not yet exposed it for lack of a good accessor name. Any suggestions? How about queryType() or requestType() and queryHost() or requestHost()? (I wouldn't go for queryDomain() because that doesn't sound fit for reverse lookups). The RFC and Wikipedia etc. seem to use both 'query' and 'request'... (...) I changed my mind about the replies after all, I find having a single QDnsReply class works well so far (as you probably noticed, I added support for MX and TXT records). Do you see a compelling reason to change this? I am happy with having one QDnsReply class; I think we can get pretty far with some special accessors for SRV and other records, and one generic accessor for simple (QString) and unsupported cases. Then whoever wants to read a currently unsupported response format needs to parse the response string himself, which IMO is not that bad. We can then in the future extend the class with more special accessors if need be. Thanks to Shane's feedback the win32 code now compiles and passes all tests. Jeremy -- Qt Developer Days 2011 – REGISTER NOW! October 24 – 26, Munich November 29 – December 1, San Francisco Learn more and Register at http://qt.nokia.com/qtdevdays2011 ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] API review for a new QDnsResolver class
On Friday, 4 de November de 2011 10:36:29 Peter Hartmann wrote: I am happy with having one QDnsReply class; I think we can get pretty far with some special accessors for SRV and other records, and one generic accessor for simple (QString) and unsupported cases. Then whoever wants to read a currently unsupported response format needs to parse the response string himself, which IMO is not that bad. We can then in the future extend the class with more special accessors if need be. There's no way to provide the unsupported formats in QString. It would need to be QByteArray and, even then, it's useless if it contains a compressed domain name. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center Intel Sweden AB - Registration Number: 556189-6027 Knarrarnäsgatan 15, 164 40 Kista, Stockholm, Sweden signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] API review for a new QDnsResolver class
On Friday, 4 de November de 2011 10:15:54 André Somers wrote: The more I think about it, the more I think it is important to fix this: who is responsible for the lifetime of the QDnsReply object? This API does not make that clear. I like the pattern in itself (also in QNAM), but I do think it would be an improvement if we were to use a shared pointer to the reply object. That at least makes clear who has ownership of the object, and prevents memory leaks when people don't realize they are supposed to delete the object. Maybe we should pattern this class against something that is already known: http://doc.qt.nokia.com/latest/q3dns.html This class is both the request and the reply and its ownership is clearly known. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center Intel Sweden AB - Registration Number: 556189-6027 Knarrarnäsgatan 15, 164 40 Kista, Stockholm, Sweden signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] API review for a new QDnsResolver class
Le Nov 4, 2011 à 11:05 AM, Thiago Macieira a écrit : On Friday, 4 de November de 2011 10:36:29 Peter Hartmann wrote: I am happy with having one QDnsReply class; I think we can get pretty far with some special accessors for SRV and other records, and one generic accessor for simple (QString) and unsupported cases. Then whoever wants to read a currently unsupported response format needs to parse the response string himself, which IMO is not that bad. We can then in the future extend the class with more special accessors if need be. There's no way to provide the unsupported formats in QString. It would need to be QByteArray and, even then, it's useless if it contains a compressed domain name. Domain name expansion is handled for known fields, win32 does it itself as far as I know, and for unix I call dn_expand. Jeremy ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
[Development] Gerrit - How to search changes?
Hi, I have checked the wiki and I am failing to find the information. I want to search for all changes to directfb and I fail to do so. I could do this by searching for directfb in messages or based on a file glob. I somehow fail... Searching by name: message:directfb does work. Searching by file: I want to use file:*directfb*, but according to this[1] I will need to watch the project, so I started to watch qt/qtbase with a file:\*directfb\* glob but I don't know how to view the result of this search/watch. how to search for files? [1] http://gerrit-documentation.googlecode.com/svn/Documentation/2.2.0/user-search.html ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Gerrit - How to search changes?
On 11/04/11 14:29, ext Holger Hans Peter Freyther wrote: I want to use file:*directfb*, but according to this[1] I will need to watch the project, correct. the search function generally sucks. so I started to watch qt/qtbase with a file:\*directfb\* glob but this won't work. this must be a regexp, not a glob. e.g., i have: qt-creator/qt-creator file:^src/(shared/proparser|plugins/(debugger|qt4projectmanager)) I don't know how to view the result of this search/watch. you don't. you need to enable notifications for new changes on these watches. how to search for files? you don't. yes, this sucks. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] API review for a new QDnsResolver class
On 11/04/2011 11:47 AM, Peter Hartmann wrote: Btw. I think we need the generic accessor anyway because you never know what the format of a TXT response looks like. Q3Dns allowed that only for TXT records, but IMO it would be better to always have an accessor to the raw response data, or at least if the query type is unknown. Access to the raw response data for arbitrary records is not going to work in a cross-platform way. For instance, on Windows the result has already been digested for you and put into a suitable structure depending on the record type: http://msdn.microsoft.com/en-us/library/windows/desktop/ms682082%28v=vs.85%29.aspx Jeremy ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] API review for a new QDnsResolver class
Op 4-11-2011 20:31, Jeremy Lainé schreef:
On 11/04/2011 10:15 AM, André Somers wrote:
The more I think about it, the more I think it is important to fix this: who
is
responsible for the lifetime of the QDnsReply object?
Why not have the same pattern as QNAM for consistency?
How about fixing QNAM to solve this issue there as well then?
This API does not make that clear. I like the pattern in itself (also in
QNAM), but I do
think it would be an improvement if we were to use a shared pointer to the
reply object.
That at least makes clear who has ownership of the object, and prevents
memory leaks
when people don't realize they are supposed to delete the object.
I'm not sure I understand how using a QSharedPointer would clarify the API,
it would lead
to code like this:
void someObject::someMethod()
{
QSharedPointerQDnsReply reply = someResolver-lookupService(X, Y, Z);
connect(reply.data(), SIGNAL(finished()),
this, SLOT(replyFinished()));
}
= surprise, the reply has been deleted!
Has it? Probably not, as the lookup service is still holding on to
another shared pointer to it in order to know where to put the results
of the lookup, right? Using a shared pointer, at least you are sure the
object is valid while either the receiver or the resolver is still
interested in having the object around, and as soon as that is no longer
the case, it will be properly deleted. Using raw pointers, you can not
be sure of either.
dreaming
Would be nice by the way if connect could directly deal with QObjects in
shared pointers...
/dreaming
Alternatively, perhaps a look at QFuture would help. QFuture is another way
results that
are not yet ready are handled in Qt, but this time it is returned as a value
instead of
as a pointer. It would be nice we could come up with a single approach for
these kinds
of things and use that all over the place...
I brought this up with Thiago on IRC, as I quite like the idea of using a
QFuture. However
apparently this is not an option as QtConcurrent is optional.
Me too. My point was, that we have slightly different patters for
basically the same sort of thing in different places in Qt. QFuture is
_currently_ coupled with QtConcurrent, but is there a strong reason why
is must be? I was not privy to that IRC chat, perhaps you could tell us
the reasoning why it would not be possible?
I think the Qt API would benefit from having the same kind of pattern
for waiting for asynchronous results, whether from calculation or from
the network or from some other resource. However, at all times, I think
it must be clear who owns what object, and who is responsible for
cleaning it up.
André
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] API review for a new QDnsResolver class
On 11/04/2011 09:01 PM, Andre Somers wrote:
Op 4-11-2011 20:31, Jeremy Lainé schreef:
On 11/04/2011 10:15 AM, André Somers wrote:
The more I think about it, the more I think it is important to fix this:
who is
responsible for the lifetime of the QDnsReply object?
Why not have the same pattern as QNAM for consistency?
How about fixing QNAM to solve this issue there as well then?
This API does not make that clear. I like the pattern in itself (also in
QNAM), but I do
think it would be an improvement if we were to use a shared pointer to the
reply object.
That at least makes clear who has ownership of the object, and prevents
memory leaks
when people don't realize they are supposed to delete the object.
I'm not sure I understand how using a QSharedPointer would clarify the API,
it would lead
to code like this:
void someObject::someMethod()
{
QSharedPointerQDnsReply reply = someResolver-lookupService(X, Y,
Z);
connect(reply.data(), SIGNAL(finished()),
this, SLOT(replyFinished()));
}
= surprise, the reply has been deleted!
Has it? Probably not, as the lookup service is still holding on to
another shared pointer to it in order to know where to put the results
of the lookup, right? Using a shared pointer, at least you are sure the
object is valid while either the receiver or the resolver is still
interested in having the object around, and as soon as that is no longer
the case, it will be properly deleted. Using raw pointers, you can not
be sure of either.
Let's say that another shared pointer is indeed held by the resolver class, at
what point
should we let go of this shared pointer? The answer is probably at some point
after the
finished() signal is emitted, but when exactly?
dreaming
Would be nice by the way if connect could directly deal with QObjects in
shared pointers...
/dreaming
Yep, otherwise we're going to have some confused users!
Jeremy
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] API review for a new QDnsResolver class
On Friday, 4 de November de 2011 21:01:30 Andre Somers wrote: Me too. My point was, that we have slightly different patters for basically the same sort of thing in different places in Qt. QFuture is currently coupled with QtConcurrent, but is there a strong reason why is must be? I was not privy to that IRC chat, perhaps you could tell us the reasoning why it would not be possible? There's no reason why it has to be coupled with Concurrent. Or, to put in other words, it could be changed to work without Concurrent. However, the problem is, it is currently too tightly coupled with QtConcurrent. Unless someone is volunteering to do this work right now... -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center Intel Sweden AB - Registration Number: 556189-6027 Knarrarnäsgatan 15, 164 40 Kista, Stockholm, Sweden signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Best place for new patches, qt or qtbase?
On Friday, 4 de November de 2011 12:36:40 Chris Meyer wrote: I'm preparing some patches that help applications built on Qt get accepted into the App Store. My project is currently being built on qt/4.7 and I'll be switching to qt/4.8 when it is officially released. What is the best place to submit those merge requests? In qtbase/master, qt/4.7, qt/4.8, or somewhere else? Merge requests go to 4.8, for now. Gerrit contributions to 5.0. I'd actually prefer if you submitted the changes to 5.0 first, got them reviewed and accepted there, and only then backported to 4.8. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center Intel Sweden AB - Registration Number: 556189-6027 Knarrarnäsgatan 15, 164 40 Kista, Stockholm, Sweden signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Best place for new patches, qt or qtbase?
2011/11/4 Thiago Macieira thiago.macie...@intel.com: On Friday, 4 de November de 2011 12:36:40 Chris Meyer wrote: I'm preparing some patches that help applications built on Qt get accepted into the App Store. My project is currently being built on qt/4.7 and I'll be switching to qt/4.8 when it is officially released. What is the best place to submit those merge requests? In qtbase/master, qt/4.7, qt/4.8, or somewhere else? Merge requests go to 4.8, for now. Gerrit contributions to 5.0. I'd actually prefer if you submitted the changes to 5.0 first, got them reviewed and accepted there, and only then backported to 4.8. I will do that (submit to 5.0 first, then 4.8). I'm having trouble getting qtbase to configure... I'll post another message with that question. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
[Development] configure qtbase on mac os
I'm configuring qtbase (Mac OS 10.6.8, Xcode 4.2, and qtbase/master) using the following command: export QTDIR=/some/path/for/installation ./configure -prefix $QTDIR -opensource -no-sql-mysql -qt-libpng -qt-libjpeg -no-dbus -nomake examples -nomake demos After a while, I get an error that configure cannot determine big-endian or little-endian. However, these options seem to be required only if I'm building with the -qpa option, which should default to off. It looks like the variable $PLATFORM_MAC is not being set properly. Is this something that needs to be specified on the command line or is the machinery for automatically setting $PLATFORM_MAC broken? ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] Qt development topics in Qt Contribution Day
On 11/02/2011 07:36 AM, Quim Gil wrote: If you are planning to have Qt Project specific discussions, including those directly related with development and anything worth taking minutes and publish them, then we will gladly schedule them during that day. Watch http://wiki.qt-project.org/Qt_Contributors_Day - there are still many spots in the schedule. If you are planning to participate please register yourself at the wiki page. Thanks! -- Quim ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
