What does 1 mean? Is it EPERM?

Do you need to check if ent is not NULL? Couldn't you just call free?

Can you move the declaration of p to another line? It would help read the code later on.

I'd recommend setting result to 0 before done.

I have added some comments to the first two files but they apply to all of the files.

What does 1 mean? Is it EPERM?

It does not have any meaning that I'm aware of. There is a subtle difference between calling exit() and returning a value from main, but in libuser case it does not matter.

It looks like historically some people preferred exit() and the others return. Here I wanted to unify them but I do not dare change the returned value since people can relay on it in their scripts.

Do you need to check if ent is not NULL? Couldn't you just call free?

unfortunately yes, glib and libuser as well considers freeing NULL as an error and they print ugly assert warning.

Hi Iker, I updated the PR.

I split the declaration where I touch it with PR.

I moved the result = 0 assignment just before done, note that this is there just for readability.

I think you aren't using this variable.

I think you aren't using this variable.

It is used, It is just not visible in diff :)
You suggested to split declarations to multiple lines for better readability and I agree with that. It looks better IMO. Since I touched this declaration with NULL initialization (*ent = NULL) , I also split it into two lines. But the usage of groupEnt did not change. Please, take a look at whole file.

You are right! I thought it was a newly created variable.

LGTM! Ack to the patch.

Thank you for the review.

T.

Commit fe7c5d12 fixes this pull-request