Hi,
In data giovedì 19 settembre 2013 12:16:49, Svante Signell ha scritto:
> The patch is attached. Additionally, the freebsd patch is modified to
> include errno.h for definition of ENODATA and EPIPE. The compiler
> complains that EPIPE is not defined otherwise. Maybe this file can be
> included unconditionally, without harm. I wonder how the error codes
> are defined for freebsd if errno.h is not included?
Reviews below.
> --- a/src/csync_misc.c 2013-09-04 11:15:42.000000000 +0200
> +++ b/src/csync_misc.c 2013-09-19 07:09:54.000000000 +0200
> @@ -88,20 +88,15 @@ char *csync_get_local_username(void) {
> #endif /* NSS_BUFLEN_PASSWD */
>
> char *csync_get_user_home_dir(void) {
> - char home[PATH_MAX] = {0};
> - const char *envp;
> + static const char *home = NULL;
> struct passwd pwd;
> struct passwd *pwdbuf;
> char buf[NSS_BUFLEN_PASSWD];
> int rc;
>
> - envp = getenv("HOME");
> - if (envp != NULL) {
> - snprintf(home, sizeof(home), "%s", envp);
> - if (home[0] != '\0') {
> - return c_strdup(home);
> - }
> - }
> + home = getenv("HOME");
> + if (home)
> + return c_strdup(home);
Storing statically the result of getenv is wrong, since it points to the
global environment array which can be reallocated anytime.
Also, no need to rename envp to home, makes the change bigger for no
reason.
Furthermore, your changes are losing the check for an empty HOME.
A better change is the following, which is easier:
--- a/src/csync_misc.c
+++ b/src/csync_misc.c
@@ -88,7 +88,6 @@
#endif /* NSS_BUFLEN_PASSWD */
char *csync_get_user_home_dir(void) {
- char home[PATH_MAX] = {0};
const char *envp;
struct passwd pwd;
struct passwd *pwdbuf;
@@ -96,11 +95,8 @@
int rc;
envp = getenv("HOME");
- if (envp != NULL) {
- snprintf(home, sizeof(home), "%s", envp);
- if (home[0] != '\0') {
- return c_strdup(home);
- }
+ if (envp != NULL && envp[0] != '\0') {
+ return c_strdup(envp);
}
/* Still nothing found, read the password file */
> --- a/src/std/c_private.h
> +++ b/src/std/c_private.h
> @@ -26,6 +26,10 @@
> #include <sys/types.h>
> #include <sys/stat.h>
>
> +#ifdef __GNU__
> +#include <errno.h>
> +#endif
> +
No need to misuse __GNU__ for this, errno.h is a standard and such it
can be included anytime. In case it poses issues on Windows (sigh), you
can include it in the else of the _WIN32 block (as people in -bsd@
suggested regarding fcntl.h).
> #ifdef _WIN32
> #include <windef.h>
> #include <winbase.h>
> @@ -50,6 +54,11 @@
> #define geteuid() 0
> #endif
>
> +#ifndef ENODATA
> +#define ENODATA EPIPE
> +#endif
This may be problematic. c_copy in src/std/c_file.c uses ENODATA as
errno in a couple of error situations, but since c_copy is an exported
symbol (although its header is not installed), users of this function
would need to do this ENODATA→EPIPE mapping themselves too.
--
Pino ToscanoAttachment:
signature.asc
Description: This is a digitally signed message part.