Goodies
Goodies

Reputation: 4681

C Returning String Not Correct

I have a simple function to get the MAC address on a linux machine (assuming eth0 exists. I'll write more later on).

Side Question: Is there an interface universal across ALL Linux distros? Other than lo?

C Function:

char* getmac(){
        FILE *mf;
        mf = fopen("/sys/class/net/eth0/address", "r");
        if(!mf) exit(-1);
        char *mac;
        fgets(mac, 18, mf);
        printf("%2\n", mac);
        return mac;
}

Now, this prints out the MAC perfectly. However, when I return it, I get a totally different value.

char *m;
m = getmac();
printf("%s\n", m);

yields a completely different, mostly unreadable string. The first 2 characters are ALWAYS correct, after that, it's unreadable...


This worked like a charm:

char* getmac(){
        FILE *mf;
        mf = fopen("/sys/class/net/eth0/address", "r");
        if(!mf) exit(-1);
        char *mac;
        mac = calloc(18, sizeof(char));
        if(!mac) exit(-1);
        fgets(mac, 18, mf);
        printf("%s\n", mac);
        return mac;
}

Thanks for the answers!! Also, free'd later on.

Upvotes: 0

Views: 128

Answers (3)

Kninnug
Kninnug

Reputation: 8053

You never allocated memory for mac. Malloc it:

char * mac = malloc(18);

(char mac[18] wouldn't work since you want to return the string from the function, so it needs to live beyond the scope of getmac)

Don't forget to free it!

Alternatively, make it the caller's responsibility:

void getmac(char mac[18]){

Note that this requires you to pass an array of exactly 18 chars, i.e. not one that was malloced or isn't exactly 18 elements. So, call it with this:

char mac[18];
getmac(mac);

To allow a bit more freedom declare it with this:

void getmac(char * mac)

And make sure the caller always allocates enough memory.

As the other answers note you could also pass a size parameter, but since you know that you need a buffer of exactly 18 characters this seems a bit redundant to me. A size parameter does become important when you work with strings of variable length.

Upvotes: 1

cnicutar
cnicutar

Reputation: 182664

    char *mac;
    fgets(mac, 18, mf);

You need to assign memory to mac first, perhaps with malloc.


As jia103 points out in the comments, one way to structure your code could be:

char* getmac(char *buf, size_t size);

Then the caller can more freely decide how the contents are stored (perhaps the client isn't all that interested in malloc and would much prefer an array on the stack).

Upvotes: 2

jia103
jia103

Reputation: 1134

You're writing to memory you're not supposed to. You never initialized mac to any usable memory, so it's writing to whatever memory location it initially had. You'd catch this immediately if you have a habit of initializing your variables, but I can't remember if C allows this:

FILE* mf = NULL;
char* mac = NULL;

The reason this should get caught is because your program will fail when you try to call fgets() and pass it NULL for mac.

What you should do is provide a storage area in which to write the data so that you can get it when it comes back out.

void getmac(char* buffer,
            int   bufsize)
{
   ...
}

Then, you can call it as follows:

const int BUFSIZE = 1024;
char m[BUFSIZE] = {0};
getmac(m, BUFSIZE);
printf("%s\n", m);

Upvotes: 0

Related Questions