0
char* meminfo()
{
char* buffer = NULL;
FILE* fp = fopen("/proc/meminfo", "r");
ssize_t read;

char* json = malloc(4096);
char* original = json;
json[0] = '\0';
json = strcat(json, "{");

size_t value = 1024;

while ( (read = getline(&buffer, &value, fp)) != -1)
{
    char name[1024];
    name[0] = '\0';

    char val[1024];
    val[0] = '\0';

    sscanf(buffer, "%s:", name);
    buffer = buffer + strlen(name);
    name[strlen(name) - 1] = '\0';

    sscanf(buffer, "%s kB", val);

    json = strcat(json, "\"");
    json = strcat(json, name);
    json = strcat(json, "\": \"");
    json = strcat(json, val);
    json = strcat(json, "\", ");
}
int n = strlen(json);
json[n - 2] = '}';
json[n - 1] = '\0';

fclose(fp);
return original;
}

So I have this function that reads and generates a json string object for the data in the meminfo file. However, if I call free() on the return value of this function, I get a seg fault and I can't figure it out (I malloc the variable within the function and free it after). Any ideas?

5
  • Then, char* json; should be global
    – Chinna
    Dec 12, 2013 at 6:47
  • weird, in my case(CentOS 5.3, gcc 4.3) your code with free() works well. no seg. fault. anyway. why don't you initialize json with memset() or bzero()?
    – Jason Heo
    Dec 12, 2013 at 7:00
  • You probably overflowed the 4096 buffer size. You should program defensively and ensure you don't write more than the buffer can hold. Dec 12, 2013 at 7:12
  • @JimGarrison Yes. I agree with you. dynamically allocation (e.g firstly figure out file size) is better. but I think 4Kb is enough for /proc/meminfo
    – Jason Heo
    Dec 12, 2013 at 7:22
  • Using a difference on indexing an array in general is a dangerous construct, which should be avoided.
    – alk
    Dec 12, 2013 at 8:52

1 Answer 1

0

You are modifing where buffer points to between the successive call to getline().

buffer = buffer + strlen(name);

As memory pointed to by buffer might be reallocated by getline() during each coming iteration it shall not be modified in between.

To fix this make a backup like shown below:

char* meminfo()
{ 
    char * buffer = NULL;

    ...

    size_t value = 0;

    while ( (read = getline(&buffer, &value, fp)) != -1)
    {
      char * p = buffer; /* save the address, as buffer is modfied below. */

      ...

      buffer = p; /*restore address. */
    }

Also the code misses error checking on system calls.


Another issue is that this lines fails miserably in case getline() does not return anything:

size_t n = strlen(json); 
/* json is "{" so its length would be 1 ... */

json[n - 2] = '}'; 
/* ... so here the code would address json[-1]. Which provokes undefinded behaviour. */

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.