Thread: vasting uint8_t to char *

Hi there,

I have a data structure as given below

Code:

struct ap {
    uint16_t    ap_freq;    /* MHz */
    int8_t      ap_noise;
    int8_t      ap_rssi;
    int8_t      ap_snr;
    uint8_t     ap_bssid[WLAN_BSSID_ASCII_LEN];
    uint8_t     ap_ssid[WLAN_SSID_LEN];
    struct wlan_ap  *prev;
    struct wlan_ap  *next;
};

The ap_ssid and ap_bssid members are of type uint8_t to be compatible with the main net80211 stack of the operating system I am using (FreeBSD).

In the library I am working on, I have several strlcpy() calls that have either ap_ssid or ap_bssid is passed as the first argument.

To compile my project, I use -Werror flag of clang, and I am getting errors like

Code:

error: passing 'uint8_t [18]' to parameter of type 'char *' converts between pointers to integer types with different sign [-Werror,-Wpointer-sign]
(void)strlcpy(s->ap_ssid ,tmp_ssid, WLAN_SSID_LEN);

I can think of two workarounds to this problem;
* type casting s->ap_ssid to (char*)
* using memcpy() instead of strlcpy()

I would like to get your opinions about those approaches I listed above, or hear any better way to get rid of error messages.

Thanks in advance...

memcpy is usually very efficient.

As long as the arrays are fairly short (say less than 50 chars, though I'm not really sure, maybe up to 100 or more) then memcpy should be more efficient than strlcpy since memcpy can just go ahead and copy all of the bytes without having to check for a zero byte. Ultimately it depends on the processor instruction set (and compiler and options, of course).

The following test either copies a 50 char array using memcpy a hundred-million times or a 19 char string + it's terminating null (20 chars in all) using strncpy a hundred-million times. memcpy is 2 to 3 times faster for me. You could try it with strlcpy (I tried it and it was even worse than strncpy, but it may not be so bad on an actual bsd system).

This is without optimization since you never know what tricks are going on there (it could realize that there's no reason to copy the same thing over and over again, for instance). When I turn on full optimization, the memcpy is basically instantaneous, whatever that means, whereas the strncpy is about the same as before.

Code:

#include <stdio.h>
#include <string.h>
#include <time.h>

//#define USE_MEMCPY     // uncomment to use memcpy

int main() {

#ifdef USE_MEMCPY
    // 50 char array (including terminating \0 char)
    char src[50] = "0123456789012345678901234567890123456789012345678";
#else
    // 19 char string + terminating \0 char
    char src[50] = "0123456789012345678";
#endif
    char dst[75];

    time_t t = clock();
    for (int i = 0; i < 100000000; i++) {
#ifdef USE_MEMCPY
        memcpy(dst, src, sizeof src);   // need to give sizeof src here
                                        // dst must be equal or greater size
#else
        strncpy(dst, src, sizeof dst);  // should give sizeof dst here
#endif
    }

    printf("%0.6fs\n", (double)(clock() - t) / CLOCKS_PER_SEC);
    return 0;
}

Thanks!

I am also considering using memcpy() as a better alternative to type casting, but then I will have to do the no-sense below for almost every strlcpy() replacement

Code:

int len;
if (strlen(tmp_ssid)>WLAN_SSID_LEN)
 len = WLAN_SSID_LEN;
else
 len = strlen(tmp_ssid);

memset(s->ap_ssid, 0, WLAN_SSID_LEN); /* I do this regardless */
memcpy(s->ap_ssid ,tmp_ssid, len);
s->ap_ssid[len] = 0;

That's not necessary at all! And even if it was, tmp_ssid would be too big if it was > WLAN_SSID_LEN - 1 since there needs to be room for the terminating null-char.

And you've changed your tune somewhat. Now you seem to be saying that you need to ensure that any unused bytes in the string are zeroed. strlcpy doesn't ensure that, but strncpy does. So you should probably just use strncpy with casts.

Code:

strncpy((char)s->ap_ssid, (char)tmp_ssid, WLAN_SSID_LEN-1);
s->ap_ssid[WLAN_SSID_LEN-1] = 0;

Sorry, I did not include the full code snippet for the function....

In fact, as a matter course, I tend to nullify the buffer with memset() all the time (no matter I use memset or strlcpy())

strlcpy() also provides a more secure coding style than memset() I reckon, am I right? (so, if I use memset(), I have to do some extra bounds checking myself)

It's pointless to "nullify" the buffer as a matter of course, but do what you will. I've given my opinion.

And strlcpy and memset are two totally different functions, so I don't know what your point is there.

I am just trying to achieve the same goal by using different approaches, i.e. either by using strlcpy() or memcpy().

Thanks for the replies

Originally Posted by fnoyan

I am just trying to achieve the same goal by using different approaches, i.e. either by using strlcpy() or memcpy(

You said memset (not memcpy) in your previous post. That's what threw me.

And in the post where I offered my "final solution" (if I may), I forgot to put the asterisks in the casts, so they should be (char*) of course.

Originally Posted by Salem

> It's pointless to "nullify" the buffer as a matter of course.
According to the same link, all the octets matter, even those after the first \0.
Making sure the whole buffer is in a consistent state, even though a short SSID is written to it might be prudent.

I didn't say it was pointless in this case, but "as a matter of course" (i.e., all the time). My proferred solution zeroes the buffer using strncpy. strlcpy apparently doesn't zero the unused portion of the buffer, so the memset would be required in that case.

Thanks Salem!

>> It might be safer in the long run to implement your own wrapper function.
Seems like FreeBSD's ifconfig implementation uses a similar approach to the one you suggested and they implemented a function that handles the (char *) to uint8_t conversion issue (the get_string() function that is internally used by ifconfig utility [1])

>> Be wary that doing something along the lines of
>> memcpy(dest,src,destLen);
>> may lead to out of bound memory read accesses if src is pointing to a short string.
I was trying to figure out a solution for the same problem you pointed out with memcpy(), i.e. data overflow when src is not long enough to store all data in dst. But I will stick with strlcpy() and try to write something similar to get_string().

@ algorishm,
>> You said memset (not memcpy) in your previous post. That's what threw me.
Sorry, a typo! It should have been memcpy() as you pointed out!

Thanks for your replies!

[1] https://svnweb.freebsd.org/base/head...w=markup#l5260