Re: [ros-dev] [ros-diffs] [cwittich] 72792: [CRYPTNET] disable broken code

[Thomas Faber]

Could you elaborate on what makes it broken? Link to a bug perhaps?

On 2016-09-24 13:39, [e-mail] wrote:
> Author: cwittich
> Date: Sat Sep 24 11:39:17 2016
> New Revision: 72792
>
> URL: http://svn.reactos.org/svn/reactos?rev=72792&view=rev
> Log:
> [CRYPTNET] disable broken code
>
> Modified:
>     trunk/reactos/dll/win32/cryptnet/cryptnet_main.c
>
> Modified: trunk/reactos/dll/win32/cryptnet/cryptnet_main.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/cryptnet/cryptnet...
> ==============================================================================
> --- trunk/reactos/dll/win32/cryptnet/cryptnet_main.c	[iso-8859-1] (original)
> +++ trunk/reactos/dll/win32/cryptnet/cryptnet_main.c	[iso-8859-1] Sat Sep 24 11:39:17 2016
> @@ -1025,6 +1025,7 @@
>               components.dwUrlPathLength + 1);
>              hFile = CreateFileW(path, GENERIC_READ, FILE_SHARE_READ,
>               NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
> +#ifndef __REACTOS__
>              if (hFile == INVALID_HANDLE_VALUE)
>              {
>                  /* Try again on the current drive */
> @@ -1049,6 +1050,7 @@
>                      }
>                  }
>              }
> +#endif
>              if (hFile != INVALID_HANDLE_VALUE)
>              {
>                  if ((ret = CRYPT_GetObjectFromFile(hFile, pObject)))

[Christoph von Wittich]

It won't work for absolute paths as it will result in C:C:\folder. I 
committed a better fix.

Am 24.09.2016 um 13:56 schrieb Thomas Faber:
> Could you elaborate on what makes it broken? Link to a bug perhaps?
>
> On 2016-09-24 13:39, [e-mail] wrote:
> > Author: cwittich
> > Date: Sat Sep 24 11:39:17 2016
> > New Revision: 72792
> >
> > URL: http://svn.reactos.org/svn/reactos?rev=72792&view=rev
> > Log:
> > [CRYPTNET] disable broken code
> >
> > Modified:
> >      trunk/reactos/dll/win32/cryptnet/cryptnet_main.c
> >
> > Modified: trunk/reactos/dll/win32/cryptnet/cryptnet_main.c
> > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/cryptnet/cryptnet...
> > ==============================================================================
> > --- trunk/reactos/dll/win32/cryptnet/cryptnet_main.c	[iso-8859-1] (original)
> > +++ trunk/reactos/dll/win32/cryptnet/cryptnet_main.c	[iso-8859-1] Sat Sep 24 11:39:17 2016
> > @@ -1025,6 +1025,7 @@
> >                components.dwUrlPathLength + 1);
> >               hFile = CreateFileW(path, GENERIC_READ, FILE_SHARE_READ,
> >                NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
> > +#ifndef __REACTOS__
> >               if (hFile == INVALID_HANDLE_VALUE)
> >               {
> >                   /* Try again on the current drive */
> > @@ -1049,6 +1050,7 @@
> >                       }
> >                   }
> >               }
> > +#endif
> >               if (hFile != INVALID_HANDLE_VALUE)
> >               {
> >                   if ((ret = CRYPT_GetObjectFromFile(hFile, pObject)))
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

[Thomas Faber]

What's the symptom you're trying to fix?
Is it a problem for Wine also, or just for us? And why?

Trying to understand why we need to break Wine sync here.

On 2016-09-24 17:01, Christoph von Wittich wrote:
> It won't work for absolute paths as it will result in C:C:\folder. I 
> committed a better fix.
>
> Am 24.09.2016 um 13:56 schrieb Thomas Faber:
> > Could you elaborate on what makes it broken? Link to a bug perhaps?
> >
> > On 2016-09-24 13:39, [e-mail] wrote:
> > > Author: cwittich
> > > Date: Sat Sep 24 11:39:17 2016
> > > New Revision: 72792
> > >
> > > URL: http://svn.reactos.org/svn/reactos?rev=72792&view=rev
> > > Log:
> > > [CRYPTNET] disable broken code
> > >
> > > Modified:
> > >      trunk/reactos/dll/win32/cryptnet/cryptnet_main.c
> > >
> > > Modified: trunk/reactos/dll/win32/cryptnet/cryptnet_main.c
> > > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/cryptnet/cryptnet...
> > > ==============================================================================
> > > --- trunk/reactos/dll/win32/cryptnet/cryptnet_main.c	[iso-8859-1] (original)
> > > +++ trunk/reactos/dll/win32/cryptnet/cryptnet_main.c	[iso-8859-1] Sat Sep 24 11:39:17 2016
> > > @@ -1025,6 +1025,7 @@
> > >                components.dwUrlPathLength + 1);
> > >               hFile = CreateFileW(path, GENERIC_READ, FILE_SHARE_READ,
> > >                NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
> > > +#ifndef __REACTOS__
> > >               if (hFile == INVALID_HANDLE_VALUE)
> > >               {
> > >                   /* Try again on the current drive */
> > > @@ -1049,6 +1050,7 @@
> > >                       }
> > >                   }
> > >               }
> > > +#endif
> > >               if (hFile != INVALID_HANDLE_VALUE)
> > >               {
> > >                   if ((ret = CRYPT_GetObjectFromFile(hFile, pObject)))

[Christoph von Wittich]

I guess CreateFile in wine doesn't return the correct LastError when you 
pass an invalid path like "C:C:\Windows"
Our CreateFile returns ERROR_INVALID_NAME, wines PATH_NOT_FOUND or 
FILE_NOT_FOUND**

Guess we need a test for CreateFile...

Am 24.09.2016 um 17:10 schrieb Thomas Faber:
> What's the symptom you're trying to fix?
> Is it a problem for Wine also, or just for us? And why?
>
> Trying to understand why we need to break Wine sync here.
>
> On 2016-09-24 17:01, Christoph von Wittich wrote:
> > It won't work for absolute paths as it will result in C:C:\folder. I
> > committed a better fix.
> >
> > Am 24.09.2016 um 13:56 schrieb Thomas Faber:
> > > Could you elaborate on what makes it broken? Link to a bug perhaps?
> > >
> > > On 2016-09-24 13:39, [e-mail] wrote:
> > > > Author: cwittich
> > > > Date: Sat Sep 24 11:39:17 2016
> > > > New Revision: 72792
> > > >
> > > > URL: http://svn.reactos.org/svn/reactos?rev=72792&view=rev
> > > > Log:
> > > > [CRYPTNET] disable broken code
> > > >
> > > > Modified:
> > > >       trunk/reactos/dll/win32/cryptnet/cryptnet_main.c
> > > >
> > > > Modified: trunk/reactos/dll/win32/cryptnet/cryptnet_main.c
> > > > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/cryptnet/cryptnet...
> > > > ==============================================================================
> > > > --- trunk/reactos/dll/win32/cryptnet/cryptnet_main.c	[iso-8859-1] (original)
> > > > +++ trunk/reactos/dll/win32/cryptnet/cryptnet_main.c	[iso-8859-1] Sat Sep 24 11:39:17 2016
> > > > @@ -1025,6 +1025,7 @@
> > > >                 components.dwUrlPathLength + 1);
> > > >                hFile = CreateFileW(path, GENERIC_READ, FILE_SHARE_READ,
> > > >                 NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
> > > > +#ifndef __REACTOS__
> > > >                if (hFile == INVALID_HANDLE_VALUE)
> > > >                {
> > > >                    /* Try again on the current drive */
> > > > @@ -1049,6 +1050,7 @@
> > > >                        }
> > > >                    }
> > > >                }
> > > > +#endif
> > > >                if (hFile != INVALID_HANDLE_VALUE)
> > > >                {
> > > >                    if ((ret = CRYPT_GetObjectFromFile(hFile, pObject)))
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

[Sylvain Petreolle]

Did you check the existing tests ? Many of them are run during test runs. Kind regards, Sylvain Petreolle

Le Samedi 24 septembre 2016 17h28, Christoph von Wittich [e-mail] a écrit :

I guess CreateFile in wine doesn't return the correct LastError when you pass an invalid path like "C:C:\Windows"
 Our CreateFile returns ERROR_INVALID_NAME, wines PATH_NOT_FOUND or FILE_NOT_FOUND

Guess we need a test for CreateFile...

Am 24.09.2016 um 17:10 schrieb Thomas Faber:

What's the symptom you're trying to fix?
Is it a problem for Wine also, or just for us? And why?

Trying to understand why we need to break Wine sync here.

On 2016-09-24 17:01, Christoph von Wittich wrote:

It won't work for absolute paths as it will result in C:C:\folder. I 
committed a better fix.

Am 24.09.2016 um 13:56 schrieb Thomas Faber:

Could you elaborate on what makes it broken? Link to a bug perhaps?

On 2016-09-24 13:39, [e-mail] wrote:

Author: cwittich
Date: Sat Sep 24 11:39:17 2016
New Revision: 72792

URL: http://svn.reactos.org/svn/reactos?rev=72792&view=rev
Log:
[CRYPTNET] disable broken code

Modified:
     trunk/reactos/dll/win32/cryptnet/cryptnet_main.c

Modified: trunk/reactos/dll/win32/cryptnet/cryptnet_main.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/cryptnet/cryptnet...
==============================================================================
--- trunk/reactos/dll/win32/cryptnet/cryptnet_main.c	[iso-8859-1] (original)
+++ trunk/reactos/dll/win32/cryptnet/cryptnet_main.c	[iso-8859-1] Sat Sep 24 11:39:17 2016
@@ -1025,6 +1025,7 @@
               components.dwUrlPathLength + 1);
              hFile = CreateFileW(path, GENERIC_READ, FILE_SHARE_READ,
               NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
+#ifndef __REACTOS__
              if (hFile == INVALID_HANDLE_VALUE)
              {
                  /* Try again on the current drive */
@@ -1049,6 +1050,7 @@
                      }
                  }
              }
+#endif
              if (hFile != INVALID_HANDLE_VALUE)
              {
                  if ((ret = CRYPT_GetObjectFromFile(hFile, pObject)))

_______________________________________________
Ros-dev mailing list
[e-mail]
http://www.reactos.org/mailman/listinfo/ros-dev

_______________________________________________
Ros-dev mailing list
[e-mail]
http://www.reactos.org/mailman/listinfo/ros-dev

[Christoph von Wittich]

CreateFileA("C:C:\Windows", GENERIC_READ, FILE_SHARE_READ,
              NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
     ok( GetLastError() == ERROR_INVALID_NAME, "Wrong LastError %d, 
expected ERROR_INVALID_NAME\n", GetLastError() );

Our CreateFile is right...

Am 24.09.2016 um 17:22 schrieb Christoph von Wittich:
> I guess CreateFile in wine doesn't return the correct LastError when 
> you pass an invalid path like "C:C:\Windows"
> Our CreateFile returns ERROR_INVALID_NAME, wines PATH_NOT_FOUND or 
> FILE_NOT_FOUND**
>
> Guess we need a test for CreateFile...
>
> Am 24.09.2016 um 17:10 schrieb Thomas Faber:
> > What's the symptom you're trying to fix?
> > Is it a problem for Wine also, or just for us? And why?
> >
> > Trying to understand why we need to break Wine sync here.
> >
> > On 2016-09-24 17:01, Christoph von Wittich wrote:
> > > It won't work for absolute paths as it will result in C:C:\folder. I
> > > committed a better fix.
> > >
> > > Am 24.09.2016 um 13:56 schrieb Thomas Faber:
> > > > Could you elaborate on what makes it broken? Link to a bug perhaps?
> > > >
> > > > On 2016-09-24 13:39,[e-mail]  wrote:
> > > > > Author: cwittich
> > > > > Date: Sat Sep 24 11:39:17 2016
> > > > > New Revision: 72792
> > > > >
> > > > > URL:http://svn.reactos.org/svn/reactos?rev=72792&view=rev
> > > > > Log:
> > > > > [CRYPTNET] disable broken code
> > > > >
> > > > > Modified:
> > > > >       trunk/reactos/dll/win32/cryptnet/cryptnet_main.c
> > > > >
> > > > > Modified: trunk/reactos/dll/win32/cryptnet/cryptnet_main.c
> > > > > URL:http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/cryptnet/cryptnet...
> > > > > ==============================================================================
> > > > > --- trunk/reactos/dll/win32/cryptnet/cryptnet_main.c	[iso-8859-1] (original)
> > > > > +++ trunk/reactos/dll/win32/cryptnet/cryptnet_main.c	[iso-8859-1] Sat Sep 24 11:39:17 2016
> > > > > @@ -1025,6 +1025,7 @@
> > > > >                 components.dwUrlPathLength + 1);
> > > > >                hFile = CreateFileW(path, GENERIC_READ, FILE_SHARE_READ,
> > > > >                 NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
> > > > > +#ifndef __REACTOS__
> > > > >                if (hFile == INVALID_HANDLE_VALUE)
> > > > >                {
> > > > >                    /* Try again on the current drive */
> > > > > @@ -1049,6 +1050,7 @@
> > > > >                        }
> > > > >                    }
> > > > >                }
> > > > > +#endif
> > > > >                if (hFile != INVALID_HANDLE_VALUE)
> > > > >                {
> > > > >                    if ((ret = CRYPT_GetObjectFromFile(hFile, pObject)))
> >
> > Ros-dev mailing list
> > [e-mail]
> > http://www.reactos.org/mailman/listinfo/ros-dev
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

[Thomas Faber]

You couldn't have started with "this fixes the test failure in
cryptnet_winetest:cryptnet"?

How about we add a test that actually covers this broken case and shows
that this whole dance is pointless and Windows doesn't add missing
drive letters in this function (thus convincing Wine to remove this
code)? Because that's what it looks like from playing around with some
file:// URLs.

On 2016-09-25 08:32, Christoph von Wittich wrote:
>  CreateFileA("C:C:\\Windows", GENERIC_READ, FILE_SHARE_READ,
>           NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
>  ok( GetLastError() == ERROR_INVALID_NAME, "Wrong LastError %d, 
>
> expected ERROR_INVALID_NAME\n", GetLastError() );
>
> Our CreateFile is right...
>
> Am 24.09.2016 um 17:22 schrieb Christoph von Wittich:
> > I guess CreateFile in wine doesn't return the correct LastError when 
> > you pass an invalid path like "C:C:\Windows"
> > Our CreateFile returns ERROR_INVALID_NAME, wines PATH_NOT_FOUND or 
> > FILE_NOT_FOUND**
> >
> > Guess we need a test for CreateFile...
> >
> > Am 24.09.2016 um 17:10 schrieb Thomas Faber:
> > > What's the symptom you're trying to fix?
> > > Is it a problem for Wine also, or just for us? And why?
> > >
> > > Trying to understand why we need to break Wine sync here.
> > >
> > > On 2016-09-24 17:01, Christoph von Wittich wrote:
> > > > It won't work for absolute paths as it will result in C:C:\folder. I
> > > > committed a better fix.
> > > >
> > > > Am 24.09.2016 um 13:56 schrieb Thomas Faber:
> > > > > Could you elaborate on what makes it broken? Link to a bug perhaps?
> > > > >
> > > > > On 2016-09-24 13:39,[e-mail]  wrote:
> > > > > > Author: cwittich
> > > > > > Date: Sat Sep 24 11:39:17 2016
> > > > > > New Revision: 72792
> > > > > >
> > > > > > URL:http://svn.reactos.org/svn/reactos?rev=72792&view=rev
> > > > > > Log:
> > > > > > [CRYPTNET] disable broken code
> > > > > >
> > > > > > Modified:
> > > > > >       trunk/reactos/dll/win32/cryptnet/cryptnet_main.c
> > > > > >
> > > > > > Modified: trunk/reactos/dll/win32/cryptnet/cryptnet_main.c
> > > > > > URL:http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/cryptnet/cryptnet...
> > > > > > ==============================================================================
> > > > > > --- trunk/reactos/dll/win32/cryptnet/cryptnet_main.c	[iso-8859-1] (original)
> > > > > > +++ trunk/reactos/dll/win32/cryptnet/cryptnet_main.c	[iso-8859-1] Sat Sep 24 11:39:17 2016
> > > > > > @@ -1025,6 +1025,7 @@
> > > > > >                 components.dwUrlPathLength + 1);
> > > > > >                hFile = CreateFileW(path, GENERIC_READ, FILE_SHARE_READ,
> > > > > >                 NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
> > > > > > +#ifndef __REACTOS__
> > > > > >                if (hFile == INVALID_HANDLE_VALUE)
> > > > > >                {
> > > > > >                    /* Try again on the current drive */
> > > > > > @@ -1049,6 +1050,7 @@
> > > > > >                        }
> > > > > >                    }
> > > > > >                }
> > > > > > +#endif
> > > > > >                if (hFile != INVALID_HANDLE_VALUE)
> > > > > >                {
> > > > > >                    if ((ret = CRYPT_GetObjectFromFile(hFile, pObject)))