> > Only one thing -- Nalin, you'd better to define some sort of function like > log_err in other modules to do syslogging. ;) You have tons of syslog calls > in there, and in every place you call openlog(), syslog() and closelog(). BTW, I am not sure whether it is appropriate to do syslog() and especially openlog()/closelog() from PAM modules, even though lots of them do so now. openlog() messes up the existing syslogging setup an application could have made. Currently, applications don't do a second call to openlog() after authentication. The reason we don't get weird log entries because of this is that most of the syslogging done by PAM modules is to report a failure, so the application simply doesn't continue. It's probably a matter of taste, but I don't like the modules using their name as the syslog ident. I think log entries like: Aug 21 07:30:36 hostname service[PID]: module: message would be more consistent. (The "service[PID]" part depends on how the application has initialized syslog.) The only problem I see with this is that it's not possible to determine whether the application has done an openlog() call at all. Maybe libpam should provide its own logging interface which could be overriden from applications. Then the module name would be available separately from the string being logged, and the logging function could decide whether it uses it as syslog ident or as a part of the message. The default logging function provided in libpam could check a global variable to find out whether it needs to call openlog(), etc. Signed, Solar Designer
