I've always been uncomfortable with the use of getpwuid and errno. It feels gross to directly mutate a global variable even though that is what the man page for getpwuid says you are supposed to do. I think the old code was ok in terms of errno (though I was forgetting to check for NULL return values) because errno is thread-local, but I still don't like it. More worrying is that the man page says "The return value may point to a static area, and may be overwritten by subsequent calls to getpwent(3), getpwnam(), or getpwuid()." This could be an issue because user::info() is called within spawn_subshell, which itself is called right at the start of a new connection. Each connection gets its own dedicated thread, so there could potentialy be a race between two calls to user::info(). In most cases this won't be an issue since they always use the same argument (uid for the current user), but there is the potential for read/write shear on some of the char*s in the passwd struct. I doubt this would cause issues in practice since the kernel probably just re-uses the same pointers for a given user, but it could be a problem in theory, and would be if the kernel builds these strings at all dynamically.
Using the API the plunks the memory for the returned passwd right on the stack is much safer, and does not require gross looking global accesses.
I've always been uncomfortable with the use of getpwuid and errno. It feels gross to directly mutate a global variable even though that is what the man page for getpwuid says you are supposed to do. I think the old code was ok in terms of errno (though I was forgetting to check for NULL return values) because errno is thread-local, but I still don't like it. More worrying is that the man page says "The return value may point to a static area, and may be overwritten by subsequent calls to getpwent(3), getpwnam(), or getpwuid()." This could be an issue because user::info() is called within spawn_subshell, which itself is called right at the start of a new connection. Each connection gets its own dedicated thread, so there could potentialy be a race between two calls to user::info(). In most cases this won't be an issue since they always use the same argument (uid for the current user), but there is the potential for read/write shear on some of the
char*s in the passwd struct. I doubt this would cause issues in practice since the kernel probably just re-uses the same pointers for a given user, but it could be a problem in theory, and would be if the kernel builds these strings at all dynamically.Using the API the plunks the memory for the returned passwd right on the stack is much safer, and does not require gross looking global accesses.