0 
#include <pwd.h>
#include <stdio.h>

struct passwd* Getpwnam_(const char* name)
{
        static struct passwd* passwd;

        while((passwd=getpwent())!=NULL)        /* get pw entry line by line */
        {
                if(strcmp(passwd->pw_name, name)==0)    /* find the same name */
                        return passwd;
        }

        if(passwd==NULL)        /* there is no matching name */
                return NULL;
}

int
main(void)
{
        printf("%ld %ld\n", (long)(Getpwnam_("root")->pw_uid), (long)(Getpwnam_("cho")->pw_uid));
}

On the above code, when I use main functions like:

printf("%ld\n", (long)(Getpwnam_("root")->pw_uid));
printf("%ld\n", (long)(Getpwnam_("cho")->pw_uid));

It is well operated. But, when I use one printf() with two Getpwnam_() as arguments, I get a segmentation fault. I think there's no problem in my code operation.

But, why this gives me an segmentation fault??

Improve this question
11
  • does user cho exisst on your system? Commented May 5, 2016 at 6:25
  • @fluter: Yes, sure Commented May 5, 2016 at 6:26
  • Are you sure that Getpwnam_() returns you non-NULL in both calls? I believe you should get NULL for the root password if you run your program as a non-root user Commented May 5, 2016 at 6:27
  • @Michael: Yse, the output is 0 \n 1000 when I use command below one, not above one` and I'm using root account. Commented May 5, 2016 at 6:30
  • Technically, you have a path out of your function without a return. In practice, the only reason for escaping the loop is covered by the if condition, so it always returns a value, but the test is unnecessary and removing the test avoids having the compiler complain. Commented May 5, 2016 at 6:31

3 Answers 3

Reset to default
3

One problem is that you're trying to keep two password entries live at the same time. A subsequent call to getpwent may overwrite the previously returned info. So you need to finish processing the returned info before calling getpwent again. If necessary, make a copy of the fields that you need.

Also, there is no need to declare the pointer to be static, since you aren't returning its address.

(The other problem was mentioned by atturri, which is that you weren't rewinding to the beginning of the passwords with setpwent between calls. This would have been clearer if the code had checked for a NULL return before attempting to reference the password fields.)

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

Comments

2

You need to rewind he password database with setpwent() between the calls to Getpwnam_().

Let's say your application calls Getpwnam_("cho") first. If in the database "root" is before "cho", in the search in one point getpwent() will return "root" but your search will discard it as it's different from "cho". Later on getpwent() will return "cho" which is the valid result.

If next your application calls Getpwnam_("root"), getpwent() will start returning entries from the point that was left on the last call, which in the database is beyond both "root" and "cho". As "root" will not be returned anymore, the search will not get the result and you'll get a null pointer which crashes the program.

Improve this answer

4 Comments

I found the problem. The reason why is that what atturri just said. When the first call of Getpwnam_("cho"), give right value. But w/o endpwent()or setpwent(). The second call of Getpwnam_() will return NULL. So outputing NULL with %ld gives me an error
@atturri: Why Getpwnam_() call in each printf() reset the entry searching line?? I mean the two call of Getpwnam_() in one printf() is not reset entry searching line. I mean when first call of Getpwnam_("cho"), it will return value. But, after call of Getpwnam_("root"), it will return NULL value because of no entry for root below thecho. But, why each printf() call search the entry at the first which doesn't make segmentation fault?
@A.Cho Sorry I'm not sure I understand. You need to search the whole database from the start in each of your while loops in order to make sure that the user is found. You can rewind the database inside Getpwnam_(), or outside. And also note that the rest of the answers point out another problem you have with your code.
Sry, I got misunderstanding on my code. Nevermind what I said. Thank you.
2

The problem here is most likely related to the fact getpwent returns a pointer to a (possibly static) memory area which it manages. So basically when you get the return value you must use it BEFORE making another call to getpwent. Because the 2nd call could overwrite or even free the area returned to by the previous call.

From the man page:

The return value may point to a static area, and may be overwritten by subsequent calls to getpwent(), getpwnam(3), or getpwuid(3). (Do not pass the returned pointer to free(3).)

So two separate prints work, because you use the first one before the 2nd call. But putting both in the same print means the 2nd call invalidates the pointer returned by the 1st call, but then print tries to use both ponters. Even if you didn't get a seg fault, it would most likely not produce correct output for the first user.

Instead you will need to change your Getpwnam_ to copy the data you need into a freshly allocated object, and return a pointer to that. NOTE: The copying will need to be "recursive" if you want some of the strings too.

Improve this answer

Comments

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.