> From bad45d57b816c3734bcbf5f92a707fc1ff112c1c Mon Sep 17 00:00:00 2001 From: Richard Jones&lt;rjones(a)redhat.com&gt; Date: Mon, 30 Nov 2009 13:55:04 +0000 Subject: [PATCH 1/5] daemon error handling: Clear errno before calling stub functions. This just ensures that we accurately report errors, even if our error path code doesn't set errno. We won't end up with a bogus errno left over from a previous call. --- daemon/proto.c | 17 ++++++++++++++++- 1 files changed, 16 insertions(+), 1 deletions(-) diff --git a/daemon/proto.c b/daemon/proto.c index 2231037..e3ff9e9 100644 --- a/daemon/proto.c +++ b/daemon/proto.c @@ -29,6 +29,10 @@ #include<rpc/types.h> #include<rpc/xdr.h> +#ifdef HAVE_WINDOWS_H +#include<windows.h> +#endif + #include "c-ctype.h" #include "ignore-value.h" @@ -137,9 +141,20 @@ main_loop (int _sock) goto cont; } - /* Now start to process this message. */ proc_nr = hdr.proc; serial = hdr.serial; + + /* Clear errors before we call the stub functions. This is just + * to ensure that we can accurately report errors in cases where + * error handling paths don't set errno correctly. + */ + errno = 0; +#ifdef WIN32 + SetLastError (0); + WSASetLastError (0); +#endif + + /* Now start to process this message. */ dispatch_incoming_message (&xdr); /* Note that dispatch_incoming_message will also send a reply. */ -- 1.6.5.2

On 30/11/09 14:42, Richard W.M. Jones wrote: >> From 23452a49f51458ca39ca4c2c953169f32ed67067 Mon Sep 17 00:00:00 2001 > From: Richard Jones&lt;rjones(a)redhat.com&gt; > Date: Fri, 27 Nov 2009 13:54:11 +0000 > Subject: [PATCH 2/5] daemon error handling: fix case_sensitive_path for Windows. > > --- > daemon/realpath.c | 12 ++++++++++++ > 1 files changed, 12 insertions(+), 0 deletions(-) > > diff --git a/daemon/realpath.c b/daemon/realpath.c > index e6c81ef..0edb1d0 100644 > --- a/daemon/realpath.c > +++ b/daemon/realpath.c > @@ -69,6 +69,7 @@ do_realpath (const char *path) > char * > do_case_sensitive_path (const char *path) > { > +#ifndef WIN32 > char ret[PATH_MAX+1] = "/"; > size_t next = 1; > int fd_cwd; > @@ -196,4 +197,15 @@ do_case_sensitive_path (const char *path) > error: > close (fd_cwd); > return NULL; > +#else /* WIN32 */ > + /* On Win32 paths are always handled case insensitively, so there is > + * no need for this function to modify the path in any way. > + */ > + char *ret = strdup (path); > + if (ret == NULL) { > + reply_with_perror ("strdup"); > + return NULL; > + } > + return ret; > +#endif /* WIN32 */ > } > -- 1.6.5.2 I'm not convinced by this. I know this call isn't required on Windows. However, making it a no-op on a Windows appliance is one of those minor, unexpected behaviour differences which is going to bite you in the arse later. For example, storing the output of case_sensitive_path for later use by a Linux appliance. It is feasible to keep the behaviour on Windows?

+/* Returns: + * 0 if everything was OK, + * -1 for a general error (sets errno), + * -2 if an existing path element was not a directory. + */ static int recursive_mkdir (const char *path) { @@ -130,10 +135,7 @@ recursive_mkdir (const char *path) if (errno == EEXIST) { /* Something exists here, might not be a dir. */ r = lstat (path,&buf); if (r == -1) return -1; - if (!S_ISDIR (buf.st_mode)) { - errno = ENOTDIR; - return -1; - } + if (!S_ISDIR (buf.st_mode)) return -2; return 0; /* OK - directory exists here already. */ }

On Mon, Nov 30, 2009 at 03:47:53PM +0000, Matthew Booth wrote: > On 30/11/09 14:47, Richard W.M. Jones wrote: >> +/* Returns: >> + * 0 if everything was OK, >> + * -1 for a general error (sets errno), >> + * -2 if an existing path element was not a directory. >> + */ >> static int >> recursive_mkdir (const char *path) >> { >> @@ -130,10 +135,7 @@ recursive_mkdir (const char *path) >> if (errno == EEXIST) { /* Something exists here, might not be a dir. */ >> r = lstat (path,&buf); >> if (r == -1) return -1; >> - if (!S_ISDIR (buf.st_mode)) { >> - errno = ENOTDIR; >> - return -1; >> - } >> + if (!S_ISDIR (buf.st_mode)) return -2; >> return 0; /* OK - directory exists here already. */ >> } >> > > If you made this return -ENOTDIR it would be slightly less magic when > used later. Unfortunately -ENOTDIR might be -1 ...