Resources cleanup upon using received data

[Dan Williams]

On Fri, 2015-06-12 at 17:24 +0200, ferran wrote:
> Hello everyone,
> I have some issues at managing resources used to query wireless
> statistics, I hope you can help me.
> 
> Currently I successfully query cfg80211 to get statistics about wireless
> devices such as tx bitrate, every 10 seconds. My problem is that I have
> been unable to do this without leaking file descriptors and memory. The
> code I use is inherited and I am trying to fix it, but I can't manage to
> release the resources without losing the retrieved data. Until now, I
> have read the documentation, compared with implementation examples found
> here and there, and consulted frequently the doxygen reference. However,
> I still need some more help.
> 
> Issue 1: 'nl_recvmsgs_default()' will block until receiving a message if
> socket is configured as blocking (default), but won't wait for the
> callback to finish, right? So how could I possibly release the resources
> just after nl_recvmsgs if the callback needs the received data?

nl_recvmsgs_default() will block and process messages as long as (a)
there are messages in the receive buffer and (b) your callback returns
NL_SKIP.  It will wait for your callback to finish before proceeding.

One thing I notice: there isn't a lot of sense in doing all the setup
every 10 seconds.  You might as well just do the setup once and then
every 10s do the nl_msg_alloc/send/recv.  

> Issue 2: I have sumarized  the code so that you can check the run flow
> and the memory management, it's posted in pastebin:
> http://pastebin.com/EYADmDpy
> Could you have a look to it, please? It's less than 100 lines, comments
> included. How should I release the resources used by the nl socket and
> nl message?

The code you've got isn't freeing either 'msg' or 'sk', so you're
leaking both of those every 10 seconds.  How about something like this;
alternatively you can just make sure that both the normal and
nla_put_failure paths in get_wireless_stats() free both 'msg' and 'sk'.

//// ----------- caller.c ------------------
/* includes */
int main ()
    struct wireless_stats ofs;
    struct nl_sock *sk;
    int family_id;
    int err;

    sk = nl_socket_alloc();
    genl_connect(sk);

    // WARNING, the docs say we must call genl_family_put() later and we don't!    
    family_id = genl_ctrl_resolve(sk, "nl80211");
    
    nl_socket_modify_cb(sk, NL_CB_VALID, NL_CB_CUSTOM,
        stats_handler, NULL);

    while (true)
    {
        get_wireless_stats(sk, &ofs);
        /* use retrieved data */
    
        sleep_seconds(10);
    }

    nl_socket_free(sk);
return 0;

//// --------- wireless_stats.c -----------

void get_wireless_stats(struct nl_socket *sk, struct wireless_stats *ofs)
{
    struct nl_msg *msg;
    int err;

    shared_stats = ofs;
    
    msg = nlmsg_alloc();
    genlmsg_put(msg, 0, 0, family_id, 0, NLM_F_DUMP, NL80211_CMD_GET_STATION, 0;
    NLA_PUT_U32(msg, NL80211_ATTR_IFINDEX, if_nametoindex("mesh0"));
    err = nl_send_auto_complete(sk, msg);
    nl_recvmsgs_default(sk);
    nlsmg_free(msg);
    return 0;
    
    // Not sure who comes here, but it isn't part of the success workflow, and is required at compilation time.
    // -> the NLA_PUT_U32 macro has a 'goto nla_put_failure' in case there isn't enough memory
nla_put_failure:
    nlmsg_free(msg);
    return err;
}

<no changes to stats_handler>