Read /proc program

Question

I am trying to write a script that has a function to get process details.

So far I have

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

char* getField(FILE* file, char* prop, int len){
 char line[100], *p;

 while(fgets(line, 100, file)) {
     if(strncmp(line, prop, len) != 0)
           continue;

     p = line + len + 1;
     while(isspace(*p)) ++p;

     break;
 }

 return p;
}

int main(int argc, char *argv[]) {

  char tgid[40], status[40], *s, *n;
  FILE* statusf;

  printf("Please Enter PID\n");

  if (fgets(tgid, sizeof tgid, stdin)) {
    //Remove new line 
    strtok(tgid, "\n");
    snprintf(status, 40, "/proc/%s/status", tgid);

    statusf = fopen(status, "r");
    if(!statusf){
      perror("Error");
      return 0;
    }

    s = getField(statusf, "State:", 6);
    n = getField(statusf, "Name:", 5);

    printf("State: %s\n", s);
    printf("Name: %s\n", n);

  }else{
    printf("Error on input");
  }

  fclose(statusf);
  return 1;
}

I am still finding the pointers and memory a bit fuzzy. When I run this script without

n = getField(statusf, "Name:", 5);

I get the correct output ( eg. S - Sleeping );

But when I call the function to get the process name I seem to get the same out put for both eg.

State: ntary_ctx Name: ntary_ctx

And that isn't even the right name. I think the issue must be the functions variables are keeping there value. But I thought that when a function return its memory is then pop off the stack.

@rtur thanks , just changed the title. I have a lot to learn — David Kirwan, Feb 24 '17 at 12:36
Compile with all warnings & debug info (`gcc -Wall -g`) then use the debugger (`gdb`). BTW, read more about C (e.g. [here](http://en.cppreference.com/w/c)) and consider using `scanf` to read some `int` pid. — Basile Starynkevitch, Feb 24 '17 at 12:37
@BasileStarynkevitch Thanks , yea I really do need to read alot more. I tend to try just dive into it , which isnt the right way to go about it. — David Kirwan, Feb 24 '17 at 12:41
As soon as you return from getfield the pointer to the old line becomes invalid because what it points to is gone. — stark, Feb 24 '17 at 12:43
the question is about a classic mistake, but respects [ask]. — Jean-François Fabre, Feb 24 '17 at 12:48
C is not a good (euphemism!) language to learn by trial&error. Get a good book. — too honest for this site, Feb 24 '17 at 12:52

score 4 · Accepted Answer · edited May 23 '17 at 12:09

Code is retuning a pointer to a local variable.
That is not valid - it is undefined behavior (UB). @stark
That explains the "I seem to get the same out put for both", as one possible UB is that the same buffer is re-used. Another possibility is that code crashes, amongst other candidates.

// Bad code
char* getField(FILE* file, char* prop, int len){
 char line[100], *p;
 ...
 p = line + len + 1;
 ...
 return p;  // `p` points to `line[]`
}

Code needs to make a copy. Could do this by allocation or passing in a destination as shown below.

char* getField(FILE* file, char *dest, const char* prop, int len){
  if (problem) return NULL;
  ...
  return strcpy(dest, p);
}

// Example call
char prop_state[100];
if (getField(statusf, prop_state, "State:", 6)) Success();
else Handle_Problem();
...
char prop_name[100];
if (getField(statusf, prop_name, "Name:", 6)) Success();
...

Better code would pass in the size of dest so getField() could handle that

char* getField(FILE* file, char *dest, size_t size, const char* prop, int len){
  ... 
  if (strlen(p) >= size) return NULL;  // Not enough room
  return strcpy(dest, p);
}

// usage
if (getField(statusf, prop_state, sizeof prop_state, "State:", 6)) Success();
...

@davidKirwan Note: Suggest passing the prompt to `getField()` so it may re-prompt as needed. The `continue` does not provide feedback to the user. [Example](http://stackoverflow.com/a/41146306/2410359) — chux - Reinstate Monica, Feb 24 '17 at 12:56

score 1 · Answer 2 · edited May 23 '17 at 12:00

1

One problem is that in getField() you're returning p which is a pointer to line plus some offset. But line is a local variable in that function so it's out of scope when the function terminates. The answer is a great explanation.

As a first step, you could make it static char line[100] to be allowed to use the pointer after the function returns, but then the second call still would overwrite what you have read by the fist call. So the best ways is to pass an additional buffer for the value:

char* getField(FILE* file, char* prop, int len, char *value){
     char line[100], *p; // now it's ok
     ... // everything at it is now before return
     strcpy( value, p );
     return value;
}

and in main() you'll have two different buffers

char name[100], state[100];  // at least same size as line - length of label
....
s = getField(statusf, "State:", 6, state);
n = getField(statusf, "Name:", 5, name);

edited May 23 '17 at 12:00

Community

1
1

answered Feb 24 '17 at 12:45

Ingo Leonhardt

9,435
2
24
33

Static does not help because he calls the function multiple times. – stark Feb 24 '17 at 12:47
@stark oops, you're right, ' haven't looked at that. I'll correct the answer – Ingo Leonhardt Feb 24 '17 at 12:49
Nice answer that shows multiple calls `getField()`. – chux - Reinstate Monica Feb 24 '17 at 13:45

score 0 · Answer 3 · answered Feb 24 '17 at 12:38

0

If you use that approach you need to close and reopen the file on each read of the field, or at leat rewind to the start. It's not really a good way for writing a C input file parser but it will do for a short file and a quick program.

answered Feb 24 '17 at 12:38

Malcolm McLean

6,258
1
17
18

Ahh okay. Yea this is probably all sorts of wrong, i am going to do alot a research into proper C programming practices. – David Kirwan Feb 24 '17 at 12:43
Generally you'll declare a struct with all the options, then write a function which fills it in from the config file. So the config file is read only once. – Malcolm McLean Feb 24 '17 at 12:53

Read /proc program

3 Answers3