C Returning String Not Correct

Question

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.

Don't write "answered" into the title, accept the relevant answer and the question gets marked as answered, the answerer gets additional point and the question disappears from the unanswered questions section. — datenwolf
– datenwolf, Commented Feb 12, 2014 at 23:05
My apologies. At the time, the time threshold of accepting an answer had not yet passed. Thank you for the tip! — Goodies
– Goodies, Commented Feb 13, 2014 at 1:20

cnicutar · Accepted Answer · 2014-02-12 23:02:49Z

2

    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).

edited Feb 12, 2014 at 23:02

answered Feb 12, 2014 at 22:51

cnicutar

184k26 gold badges378 silver badges398 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

jia103 Over a year ago

malloc and free are great, but if you have control of the method, consider taking in a buffer provided by the client. Then, the client is responsible for both the allocation and the deallocation, and is less likely to forget to free.

Kninnug · Accepted Answer · 2014-02-12 23:06:42Z

1

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.

edited Feb 12, 2014 at 23:06

answered Feb 12, 2014 at 22:53

Kninnug

8,1121 gold badge34 silver badges45 bronze badges

3 Comments

Goodies Over a year ago

Will update :) Worked like a charm with calloc (string terminates at \x00 which is initialized by calloc).

jia103 Over a year ago

malloc/calloc and free are great, but if you have control of the method, consider taking in a buffer provided by the client. Then, the client is responsible for both the allocation and the deallocation, and is less likely to forget to free.

Kninnug Over a year ago

@jia103 Added malloc-less alternatives

jia103 · Accepted Answer · 2014-02-12 22:58:30Z

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);

Collectives™ on Stack Overflow

C Returning String Not Correct

3 Answers 3

1 Comment

3 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

3 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related