[PATCH] Fix errno codes set by opendir() in case of problems with the path argument

[PATCH] Fix errno codes set by opendir() in case of problems with the path argument

[Oleg Kravtsov]

[-- Attachment #1: Type: text/plain, Size: 475 bytes --]

Currently cygwin has a problem with errno code set by opendir() 
function. It always sets errno to ENOENT.
After applying the path opendir() sets errno to 'ENAMETOOLONG' when path 
or a path component is too long,
'ELOOP' when a loop of symbolic links exits in the path.

Best regards,
Oleg

2014-02-18  Oleg Kravtsov <Oleg.Kravtsov@oktetlabs.ru>

        * dir.cc (opendir): Set errno code depending on the type of an error
        instead of always setting it to ENOENT.



[-- Attachment #2: opendir_errno_fix.patch --]
[-- Type: text/x-diff, Size: 858 bytes --]

Index: cygwin/dir.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/dir.cc,v
retrieving revision 1.136
diff -u -p -r1.136 dir.cc
--- cygwin/dir.cc	31 Jan 2014 19:27:26 -0000	1.136
+++ cygwin/dir.cc	3 Mar 2014 18:33:55 -0000
@@ -57,7 +57,16 @@ opendir (const char *name)
 
   fh = build_fh_name (name, PC_SYM_FOLLOW);
   if (!fh)
-    res = NULL;
+    {
+      res = NULL;
+      goto done;
+    }
+
+  if (fh->error ())
+    {
+      debug_printf ("got %d error from build_fh_name", fh->error ());
+      set_errno (fh->error ());
+    }
   else if (fh->exists ())
     res = fh->opendir (-1);
   else
@@ -71,6 +80,8 @@ opendir (const char *name)
   /* Applications calling flock(2) on dirfd(fd) need this... */
   if (res && !fh->nohandle ())
     fh->set_unique_id ();
+
+done:
   return res;
 }
 

Re: [PATCH] Fix errno codes set by opendir() in case of problems with the path argument

[Christopher Faylor]

On Mon, Mar 03, 2014 at 10:34:59PM +0400, Oleg Kravtsov wrote:
>Currently cygwin has a problem with errno code set by opendir() 
>function. It always sets errno to ENOENT.
>After applying the path opendir() sets errno to 'ENAMETOOLONG' when path 
>or a path component is too long,
>'ELOOP' when a loop of symbolic links exits in the path.
>
>Best regards,
>Oleg
>
>2014-02-18  Oleg Kravtsov <Oleg.Kravtsov@oktetlabs.ru>
>
>        * dir.cc (opendir): Set errno code depending on the type of an error
>        instead of always setting it to ENOENT.

Thanks for the patch but I don't see any reason for a goto here.  Also
you seem to be skipping over a free which could result in a memory leak.

I think the below should do the same thing without those limitations.

Does it work for you?

cgf

Index: dir.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/dir.cc,v
retrieving revision 1.136
diff -d -u -p -r1.136 dir.cc
--- dir.cc	31 Jan 2014 19:27:26 -0000	1.136
+++ dir.cc	3 Mar 2014 19:31:30 -0000
@@ -58,6 +58,11 @@ opendir (const char *name)
   fh = build_fh_name (name, PC_SYM_FOLLOW);
   if (!fh)
     res = NULL;
+  else if (fh->error ())
+    {
+      set_errno (fh->error ());
+      res = NULL;
+    }
   else if (fh->exists ())
     res = fh->opendir (-1);
   else

Re: [PATCH] Fix errno codes set by opendir() in case of problems with the path argument

[Oleg Kravtsov]

On Mon, 3 Mar 2014 14:32:01 -0500, Christopher Faylor wrote:
> On Mon, Mar 03, 2014 at 10:34:59PM +0400, Oleg Kravtsov wrote:
>> Currently cygwin has a problem with errno code set by opendir()
>> function. It always sets errno to ENOENT.
>> After applying the path opendir() sets errno to 'ENAMETOOLONG' when path
>> or a path component is too long,
>> 'ELOOP' when a loop of symbolic links exits in the path.
>>
>> 2014-02-18  Oleg Kravtsov <Oleg.Kravtsov@oktetlabs.ru>
>>
>>        * dir.cc (opendir): Set errno code depending on the type of an error
>>        instead of always setting it to ENOENT.

> Thanks for the patch but I don't see any reason for a goto here.  Also
> you seem to be skipping over a free which could result in a memory leak.
Actually I wanted opendir() to be in sync with other functions (mkdir, rmdir), i.e. use the same style.
There is no memory leak in the proposed patch, but it does not matter, your patch is also a valid one, though not comply the style used in other functions.

> I think the below should do the same thing without those limitations.
Yes, please fix it in a way you think is the most preferable.

> Does it work for you?
Yes, thanks.

Oleg