Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 47936dd7 authored by Ken Chen's avatar Ken Chen
Browse files

Fix -Wnullable-to-nonnull-conversion errors 

Aosp/2522835 adds _Nullable/_Nonnull annotations which causes build
break in DNS resolver module and tests.

This commit tries to:
1. Check if a nullable variable is non-null before passing it to a
   function with a non-null parameter annotation. This is necessary
   because passing null pointers to memcpy(), strlen(), strdup(), etc is
   undefined behavior.
2. Add a hacky way to avoid -Wnullable-to-nonnull-conversion errors.
   Compiler is not smart enough to know if a variable is guaranteed
   non-null from context.

Bug: 278513807
Test: presubmit
Change-Id: I2bcf887e31a2d9716c1da75431280ec29d4e5c34
parent 1dc98ce0

DnsProxyListener.cpp

+23 −7
Original line number Diff line number Diff line
@@ -455,8 +455,11 @@ void logDnsQueryResult(const addrinfo* res) { 
    LOG(DEBUG) << __func__ << ": DNS records:";

    for (ai = res, i = 0; ai; ai = ai->ai_next, i++) {

        if ((ai->ai_family != AF_INET) && (ai->ai_family != AF_INET6)) continue;

        // Reassign it to a local variable to avoid -Wnullable-to-nonnull-conversion on calling

        // getnameinfo.

        const sockaddr* ai_addr = ai->ai_addr;

        char ip_addr[INET6_ADDRSTRLEN];

        int ret = getnameinfo(ai->ai_addr, ai->ai_addrlen, ip_addr, sizeof(ip_addr), nullptr, 0,

        int ret = getnameinfo(ai_addr, ai->ai_addrlen, ip_addr, sizeof(ip_addr), nullptr, 0,

                              NI_NUMERICHOST);

        if (!ret) {

            LOG(DEBUG) << __func__ << ": [" << i << "] " << ai->ai_flags << " " << ai->ai_family
@@ -573,7 +576,10 @@ bool synthesizeNat64PrefixWithARecord(const netdutils::IPPrefix& prefix, addrinf 
        sa->ai_next = nullptr;



        if (cur4->ai_canonname != nullptr) {

            sa->ai_canonname = strdup(cur4->ai_canonname);

            // Reassign it to a local variable to avoid -Wnullable-to-nonnull-conversion on calling

            // strdup.

            const char* ai_canonname = cur4->ai_canonname;

            sa->ai_canonname = strdup(ai_canonname);

            if (sa->ai_canonname == nullptr) {

                LOG(ERROR) << "allocate memory failed for canonname";

                freeaddrinfo(sa);
@@ -733,13 +739,15 @@ static bool sendhostent(SocketClient* c, hostent* hp) { 
    bool success = true;

    int i;

    if (hp->h_name != nullptr) {

        success &= sendLenAndData(c, strlen(hp->h_name) + 1, hp->h_name);

        const char* h_name = hp->h_name;

        success &= sendLenAndData(c, strlen(h_name) + 1, hp->h_name);

    } else {

        success &= sendLenAndData(c, 0, "") == 0;

    }



    for (i = 0; hp->h_aliases[i] != nullptr; i++) {

        success &= sendLenAndData(c, strlen(hp->h_aliases[i]) + 1, hp->h_aliases[i]);

        const char* h_aliases = hp->h_aliases[i];

        success &= sendLenAndData(c, strlen(h_aliases) + 1, hp->h_aliases[i]);

    }

    success &= sendLenAndData(c, 0, "");  // null to indicate we're done
@@ -782,7 +790,12 @@ static bool sendaddrinfo(SocketClient* c, addrinfo* ai) { 
    }



    // strlen(ai_canonname) and ai_canonname.

    if (!sendLenAndData(c, ai->ai_canonname ? strlen(ai->ai_canonname) + 1 : 0, ai->ai_canonname)) {

    int len = 0;

    if (ai->ai_canonname != nullptr) {

        const char* ai_canonname = ai->ai_canonname;

        len = strlen(ai_canonname) + 1;

    }

    if (!sendLenAndData(c, len, ai->ai_canonname)) {

        return false;

    }
@@ -1401,14 +1414,17 @@ void DnsProxyListener::GetHostByAddrHandler::doDns64ReverseLookup(hostent* hbuf, 
        resolv_gethostbyaddr(&v4addr, sizeof(v4addr), AF_INET, hbuf, buf, buflen, &mNetContext, hpp,

                             event);

        endQueryLimiter(uid);

        if (*hpp) {

        if (*hpp && (*hpp)->h_addr_list[0]) {

            // Replace IPv4 address with original queried IPv6 address in place. The space has

            // reserved by dns_gethtbyaddr() and netbsd_gethostent_r() in

            // system/netd/resolv/gethnamaddr.cpp.

            // Note that resolv_gethostbyaddr() returns only one entry in result.

            memcpy((*hpp)->h_addr_list[0], &v6addr, sizeof(v6addr));

            char* addr = (*hpp)->h_addr_list[0];

            memcpy(addr, &v6addr, sizeof(v6addr));

            (*hpp)->h_addrtype = AF_INET6;

            (*hpp)->h_length = sizeof(struct in6_addr);

        } else {

            LOG(ERROR) << __func__ << ": hpp or (*hpp)->h_addr_list[0] is null";

        }

    } else {

        LOG(ERROR) << __func__ << ": from UID " << uid << ", max concurrent queries reached";

getaddrinfo.cpp

+1 −3
Original line number Diff line number Diff line
@@ -672,13 +672,11 @@ static struct addrinfo* get_ai(const struct addrinfo* pai, const struct afd* afd 
    assert(afd != NULL);

    assert(addr != NULL);



    ai = (struct addrinfo*) malloc(sizeof(struct addrinfo) + sizeof(sockaddr_union));

    ai = (struct addrinfo*) calloc(1, sizeof(struct addrinfo) + sizeof(sockaddr_union));

    if (ai == NULL) return NULL;



    memcpy(ai, pai, sizeof(struct addrinfo));

    ai->ai_addr = (struct sockaddr*) (void*) (ai + 1);

    memset(ai->ai_addr, 0, sizeof(sockaddr_union));



    ai->ai_addrlen = afd->a_socklen;

    ai->ai_addr->sa_family = ai->ai_family = afd->a_af;

    p = (char*) (void*) (ai->ai_addr);

gethnamaddr.cpp

+3 −3
Original line number Diff line number Diff line
@@ -109,9 +109,9 @@ static int dns_gethtbyname(ResState* res, const char* name, int af, getnamaddr* 
        if (eom - (ptr) < (count)) goto no_recovery; \

    } while (0)



static struct hostent* getanswer(const querybuf* _Nonnull answer, int anslen,

                                 const char* _Nonnull qname, int qtype, struct hostent* hent,

                                 char* buf, size_t buflen, int* he) {

static struct hostent* getanswer(const querybuf* _Nonnull answer, int anslen, const char* qname,

                                 int qtype, struct hostent* hent, char* buf, size_t buflen,

                                 int* he) {

    const HEADER* hp;

    const uint8_t* cp;

    int n;

sethostent.cpp

+30 −7
Original line number Diff line number Diff line
@@ -113,17 +113,28 @@ int _hf_gethtbyname2(const char* name, int af, getnamaddr* info) { 
            hent.h_addrtype = hp->h_addrtype;

            hent.h_length = hp->h_length;



            HENT_SCOPY(hent.h_name, hp->h_name, ptr, len);

            if (hp->h_name == nullptr) {

                free(buf);

                return EAI_FAIL;

            }

            const char* h_name = hp->h_name;

            HENT_SCOPY(hent.h_name, h_name, ptr, len);

            for (anum = 0; hp->h_aliases[anum]; anum++) {

                if (anum >= MAXALIASES) goto nospc;

                HENT_SCOPY(aliases[anum], hp->h_aliases[anum], ptr, len);

                const char* h_alias = hp->h_aliases[anum];

                HENT_SCOPY(aliases[anum], h_alias, ptr, len);

            }

            ptr = align_ptr(ptr);

            if ((size_t)(ptr - buf) >= info->buflen) goto nospc;

        }



        if (num >= MAXADDRS) goto nospc;

        HENT_COPY(addr_ptrs[num], hp->h_addr_list[0], hp->h_length, ptr, len);

        if (hp->h_addr_list[0] == nullptr) {

            free(buf);

            return EAI_FAIL;

        }

        const char* addr = hp->h_addr_list[0];

        HENT_COPY(addr_ptrs[num], addr, hp->h_length, ptr, len);



        num++;

    }
@@ -157,8 +168,15 @@ int _hf_gethtbyname2(const char* name, int af, getnamaddr* info) { 
    }

    hp->h_addr_list[num] = NULL;



    HENT_SCOPY(hp->h_name, hent.h_name, ptr, len);



    if (hent.h_name == nullptr) {

        free(buf);

        return EAI_FAIL;

    }

    // Curly brackets are required to avoid the "bypasses variable initialization" compile error.

    {

        const char* h_name = hent.h_name;

        HENT_SCOPY(hp->h_name, h_name, ptr, len);

    }

    for (i = 0; i < anum; i++) {

        HENT_SCOPY(hp->h_aliases[i], aliases[i], ptr, len);

    }
@@ -188,8 +206,13 @@ int _hf_gethtbyaddr(const unsigned char* uaddr, int len, int af, getnamaddr* inf 
    }

    struct hostent* hp;

    int he;

    while ((hp = netbsd_gethostent_r(hf, info->hp, info->buf, info->buflen, &he)) != NULL)

        if (!memcmp(hp->h_addr_list[0], uaddr, (size_t) hp->h_length)) break;

    while ((hp = netbsd_gethostent_r(hf, info->hp, info->buf, info->buflen, &he)) != NULL) {

        if (hp->h_addr_list[0] == nullptr) continue;

        // Reassign it to a local variable to avoid -Wnullable-to-nonnull-conversion on calling

        // memcmp.

        const char* addr = hp->h_addr_list[0];

        if (!memcmp(addr, uaddr, (size_t)hp->h_length)) break;

    }

    endhostent_r(&hf);



    if (hp == NULL) {

tests/resolv_test_utils.cpp

+4 −3
Original line number Diff line number Diff line
@@ -36,9 +36,9 @@ std::string ToString(const hostent* he) { 
std::string ToString(const addrinfo* ai) {

    if (!ai) return "<null>";



    const sockaddr* ai_addr = ai->ai_addr;

    char host[NI_MAXHOST];

    int rv = getnameinfo(ai->ai_addr, ai->ai_addrlen, host, sizeof(host), nullptr, 0,

                         NI_NUMERICHOST);

    int rv = getnameinfo(ai_addr, ai->ai_addrlen, host, sizeof(host), nullptr, 0, NI_NUMERICHOST);

    if (rv != 0) return gai_strerror(rv);

    return host;

}
@@ -84,8 +84,9 @@ std::vector<std::string> ToStrings(const addrinfo* ai) { 
        return hosts;

    }

    for (const auto* aip = ai; aip != nullptr; aip = aip->ai_next) {

        const sockaddr* ai_addr = aip->ai_addr;

        char host[NI_MAXHOST];

        int rv = getnameinfo(aip->ai_addr, aip->ai_addrlen, host, sizeof(host), nullptr, 0,

        int rv = getnameinfo(ai_addr, aip->ai_addrlen, host, sizeof(host), nullptr, 0,

                             NI_NUMERICHOST);

        if (rv != 0) {

            hosts.clear();