Thread: segmentation fault

You should:

• Indent your code properly.
• #include <stdio.h> instead of "stdio.h" although #include "stdio.h" can work.
• Declare your functions prior to their point of use.
• Check that popen and fopen succeeded before proceeding to use the FILE pointers returned.
• Check that fgets returned non-NULL rather than non-EOF, and you don't need a loop for that here.

Originally Posted by sharonch

To encourage a positive discussion, I want to clean up my code a bit. I think the major issue is in fget() and I don't know what is wrong.

In post #2, I already gave you a bunch of ways to improve your program. If you had followed up, you would have corrected your program to something like this:

Code:

#include <stdio.h>

void getKey(void);

int main(void)
{
    getKey();
    return 0;
}

void getKey(void)
{
    FILE *fp, *fp2;
    char line[100] = "#the following key is for lastfirst\n";
    char content[1206];

    fp = popen("ssh-keygen -t rsa -N \'\' -q -f lastfirst", "w");
    if (!fp)
    {
        fprintf(stderr, "Could not run ssh-keygen\n");
        return;
    }

    fp2 = fopen("lastfirst.pub", "r");
    if (!fp2)
    {
        fprintf(stderr, "Could not open public key file\n");
        pclose(fp);
        return;
    }

    if (fgets(content, sizeof(content), fp2))
    {
        printf("%s", content);
    }

    fclose(fp2);
    pclose(fp);
}

> fp = popen("ssh-keygen -t rsa -N \'\' -q -f lastfirst", "w");
> fp2 = fopen("lastfirst.pub", "r");
Why exactly are you using popen here?

Are you perhaps expecting ssh-keygen to finish writing to the file (presumably called lastfirst.pub) before the actual fopen() call in the next line?

popen() is used when you want to communicate with the process you're running.
If you just want it to do it's job, and then return when it's done, then use system()

Something weird happened. I am running this code from a different directory and it is giving me segmentation fault again. Is it mean that I can't change directory in the system()?

Code:

#include <stdio.h>
int main()
{
  getKey();
  return 0;
}

void getKey()
{
  FILE *fp, *fp2;
  char line[100] = "#\n#the following key is for lastfirst\n#\n";
  char content[1206];
  system
      ("cd /home/user/.ssh;pwd | ssh-keygen -t rsa -N \'\' -q -f lastfirst",
       "w");

  fp2 = fopen("lastfirst.pub", "r");
  if (!fp2) {
    fprintf(stderr, "no xxx.pub\n");
    fclose(fp2);
    return;
  }
  if (fgets(content, sizeof(content), fp2)) {
    printf("%s", content);
  }
  fp = fopen("lastfirst.pub", "a+");
  fprintf(fp, line);
  fprintf(fp, content);
  fclose(fp2);
}

First, start compiling with more warnings enabled and pay attention to them.

Code:

$ gcc -W -Wall foo.c
foo.c: In function ‘main’:
foo.c:4:3: warning: implicit declaration of function ‘getKey’ [-Wimplicit-function-declaration]
foo.c: At top level:
foo.c:8:6: warning: conflicting types for ‘getKey’ [enabled by default]
foo.c:4:3: note: previous implicit declaration of ‘getKey’ was here
foo.c: In function ‘getKey’:
foo.c:14:7: warning: implicit declaration of function ‘system’ [-Wimplicit-function-declaration]
foo.c:27:3: warning: format not a string literal and no format arguments [-Wformat-security]
foo.c:28:3: warning: format not a string literal and no format arguments [-Wformat-security]

> foo.c:14:7: warning: implicit declaration of function ‘system’ [-Wimplicit-function-declaration]
Which is important, because you're passing it TWO PARAMETERS and it expects only one.

> foo.c:27:3: warning: format not a string literal and no format arguments [-Wformat-security]
> foo.c:28:3: warning: format not a string literal and no format arguments [-Wformat-security]
If line or content have a % character, you're screwed.

> fprintf(stderr, "no xxx.pub\n");
> fclose(fp2);

Don't try and close it if it failed to open!

> I went into the lastfirst.pub to make sure there is no %. Therefore, the code should be ok for sometimes
That's not the problem.
The problem is if you start doing the wrong thing as a matter of habit, you'll find out the hard way sometime later on in another program where it DOES matter.

I think I have correct the warning. No segmentation faults, but now the "line" and "content" is not writing to where it suppose to.

Code:

#include <stdio.h>

Code:

int main()
{
  getKey();
  return0;
}
void getKey()
{
  FILE *fp, *fp2;
  char line[100] = "#\n#the following key is for lastfirst\n#\n";
  char content[1206];
  system("cd /home/user/.ssh/;pwd | ssh-keygen -t rsa -N \'\' -q -f lastfirst");
  if(fp2 = fopen("lastfirst.pub", "r"))
    {
      for(content; !feof(fp2); fscanf(fp2, "%s", content));
      fclose(fp2);
    }else{  return;  }

  fp = fopen("lastfirst.pub", "w+");
      fprintf(fp, "%s", line);
      fprintf(fp, "%s", content);
  fclose(fp); 
return;
}