7

I'm a C# developer trying to learn C++11. I'm trying to query DNS using windns.h.

I started with DnsQuery() and read that I need to free the result records out parameter with DnsRecordListFree(). The C# way might be to use a try-finally block to ensure that I free the resource no matter what.

But I learned that there is no finally block and that windns.h really should get with the times and implement an-RAII compliant interface (as I understand the typical advice). Instead of waiting for that to happen, I tried to make an RAII wrapper class whose destructor calls DnsRecordListFree() and with a operator overload cast to get the original pointer.

But I was confused about how to appropriately use this handle or the pointer to get an out parameter. And while I researched that I learned how unique_ptr, which I'd already learned a little bit about, can be used with a custom deleter.

So here's my simple code so far. There's probably more wrong than just this, but I figure I could declare yet another PDNS_RECORD *presult and use that as the out parameter and then copy or move or otherwise assign its value into a unique_ptr, but that sounds like too much work/mess.

It seems to me that unique_ptr's internal pointer should be initialized to NULL, that I should somehow be able to pass the pointer's address into the out parameter, that DNSQuery will update the raw value, and when the unique_ptr falls out of scope in my function the DnsRecordListFree() call will be made automatically. There's just too much I don't know to figure out the right combination for the minimal correct/safe usage.

#include <iostream>
#include <fstream>
#include <memory>
#include <Windows.h>
#include <WinDNS.h>

using namespace std;

auto pdnsDeleter = [&](PDNS_RECORD *ptr){ if (ptr) DnsRecordListFree(ptr); };

int main(int argc, char **argv)
{
    cout << "Hello World\n";

    std::unique_ptr<PDNS_RECORD*, decltype(pdnsDeleter)> results(0, pdnsDeleter);

    if (DnsQuery(L"google.com", DNS_TYPE_A, DNS_QUERY_STANDARD, NULL, ??results??, NULL))
    {
        cout << "google.com -> " << ??results??;
    }

    cout << "Done\n";
    getchar();

    return 0;
}

Thanks!

ldav1s
  • 15,885
  • 2
  • 53
  • 56
Jason Kleban
  • 20,024
  • 18
  • 75
  • 125
  • Microsoft APIs are generally written to be C compatible not C++, so no RAII for them. Wrapping is the way to go. – Mark Ransom Mar 12 '14 at 20:27
  • @MarkRansom I'm not sure if I understand you. What distinction are you drawing between RAII and "wrapping"? RAII is pattern or practice of "wrapping" a resource to control its lifetime and destruction, is it not? – Iron Savior Mar 12 '14 at 20:34
  • possible duplicate of [How to use unique\_ptr with data allocated within a c library?](http://stackoverflow.com/questions/22095214/how-to-use-unique-ptr-with-data-allocated-within-a-c-library) – Mooing Duck Mar 12 '14 at 20:42
  • @IronSavior by "wrapping" I meant putting the native type inside another wrapper class. Exactly as you said. – Mark Ransom Mar 12 '14 at 20:44
  • @IronSavior when used in the context you're describing, what Mark is discussing its often called a *janitor* class, cleaning up and turning out the lights on the way out the door. They're considerably simpler to conceive than full-blown library smart pointers, which offer many other features including copy-protection, sharing and reference counting, etc. – WhozCraig Mar 12 '14 at 20:47
  • RAII says the DnsQuery should be part of the constructor for the results. Otherwise, you're just using RDID. – Adrian McCarthy Mar 12 '14 at 21:00

2 Answers2

5

You could spend all day trying to adapt the standard smart pointer, or you could just write your own. They're not hard to do, especially if you're willing to cheat and allow access to the raw pointer itself.

struct DnsRAII
{
    PDNS_RECORD p;

    DnsRAII() : p(NULL) { }
    ~DnsRAII() { if (p != NULL) DnsRecordListFree(p, DnsFreeRecordList); }
};

DnsRAII results;
if (DnsQuery(L"google.com", DNS_TYPE_A, DNS_QUERY_STANDARD, NULL, &results.p, NULL))
// ...
Mark Ransom
  • 299,747
  • 42
  • 398
  • 622
  • 1
    What about the implicit copy constructor and copy assignment? – Mooing Duck Mar 12 '14 at 20:44
  • 1
    @MooingDuck, I suppose to be complete you should make a copy constructor and assignment operator that are private so they can't be used. Otherwise this gets a whole lot more complicated. – Mark Ransom Mar 12 '14 at 20:48
  • Ok, that's helpful, but it seems so crazy to me that it's this difficult to deal with an out parameter! This situation which must come up a lot with WinAPI and other libraries must certainly benefit from a finally block over all this extra code and convention, right? – Jason Kleban Mar 13 '14 at 01:48
  • 1
    @uosɐſ by C++ standards this is not difficult. And generally once you make a class like this you use it over and over, the overhead is only incurred once - it's actually simpler than a finally block at the use site. – Mark Ransom Mar 13 '14 at 02:22
2

Unless I'm missing something (I haven't a Windows box handy to compile this, so forgive me if I am) your code is incorrect. Personally i wouldn't do this with a smart pointer, since all you're essentially using it for is a custom janitor class (and you could write one of those easy-enough).

Regardless, first, your deleter should be :

auto pdnsDeleter = [&](PDNS_RECORD ptr){ if (ptr) DnsRecordListFree(ptr, DnsFreeRecordList); };

Next, the type of your smart pointer should be:

std::unique_ptr<DNS_RECORD, decltype(pdnsDeleter)> results;

Finally, I believe your loadup of this smart pointer should be after determination the function succeeded:

PDNS_RECORD pdnsrec;
if (DnsQuery(L"google.com", DNS_TYPE_A, DNS_QUERY_STANDARD, NULL, &pdnsrec, NULL))
{
    results.reset(pdnsrec);
}

If I got that right, your deleter will be fired correctly on scope exit on the fetched chain. But again, this seems an awful lot of work for what you can effectively make simpler with your own janitor class.

WhozCraig
  • 65,258
  • 11
  • 75
  • 141
  • Thanks! So how are these options better than a possibility of finally blocks? Still what you've written does seem to be slightly nicer and shorter than a full janitor class implementation. Perhaps you could demonstrate that still simpler code? – Jason Kleban Mar 13 '14 at 01:26
  • @uosɐſ It is pretty much exactly exactly what Mark has demonstrated, which is why i didn't really do it here. To do full RAII is more involved, but so is requirement list. A way of just *knowing* something that isn't C++ -oriented will be cleaned up is the end goal you seem to want, and both will do it. Smart pointers add to that, (like unique's ability to protect against copying, or shared's ability to reference count, etc). – WhozCraig Mar 13 '14 at 03:21