August 2021 - Wine-Devel - list.winehq.org

Re: RFC: Reparse Point/NT Symlink Support [3]

by Erich E. Hoover

On Tue, Aug 31, 2021 at 10:52 AM Zebediah Figura (she/her) <zfigura(a)codeweavers.com> wrote: > ... > Is there a problem with collapse_path(), or is it just a matter of > asking for the parent of the current directory? If the latter, wouldn't > it just be a matter of storing the unresolved path in the TEB? That's necessary but not sufficient, you also need to change the path resolution. It's completely valid (and common) to pass a relative path that does something "stupid". An extension of the Linux example: === ~/tmplnk$ cd link/.. ~/tmplnk$ ls link test ~/tmplnk$ ls link/.. target === Best, Erich

Aug. 31, 2021

Re: RFC: Reparse Point/NT Symlink Support [3]

by Zebediah Figura (she/her)

On 8/31/21 11:33 AM, Erich E. Hoover wrote: > On Tue, Aug 31, 2021 at 10:29 AM Zebediah Figura (she/her) > <zfigura(a)codeweavers.com> wrote: >> ... >> Wait, but by the time we do path resolution, aren't we guaranteed to >> have an absolute path that's free of . or ..? I thought that >> RtlDosPathNameToNtPathName() got rid of all of those for us. > > Yes, that's part of the problem - if you resolve the path to the > absolute location then ".." brings you to the parent of the symlink > target. Is there a problem with collapse_path(), or is it just a matter of asking for the parent of the current directory? If the latter, wouldn't it just be a matter of storing the unresolved path in the TEB?

Aug. 31, 2021

Re: RFC: Reparse Point/NT Symlink Support [3]

by Erich E. Hoover

On Tue, Aug 31, 2021 at 10:29 AM Zebediah Figura (she/her) <zfigura(a)codeweavers.com> wrote: > ... > Wait, but by the time we do path resolution, aren't we guaranteed to > have an absolute path that's free of . or ..? I thought that > RtlDosPathNameToNtPathName() got rid of all of those for us. Yes, that's part of the problem - if you resolve the path to the absolute location then ".." brings you to the parent of the symlink target. Best, Erich

Aug. 31, 2021

Re: [PATCH] shlwapi/tests: Document where SHFormatDateTimeA() crashes for NULL pointers.

by Zebediah Figura (she/her)

On 8/31/21 7:53 AM, Francois Gouget wrote: > Knowing which Windows versions have been confirmed to crash will make it > easier to know whether that information is potentially obsolete if the > Windows behavior changes. > > Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> > --- > I chose to put the comment on the same line as the if (0) so it is > visible even with a simple source grep. > --- > dlls/shlwapi/tests/ordinal.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/dlls/shlwapi/tests/ordinal.c b/dlls/shlwapi/tests/ordinal.c > index bf43d35b1c2..15011bbdde4 100644 > --- a/dlls/shlwapi/tests/ordinal.c > +++ b/dlls/shlwapi/tests/ordinal.c > @@ -1674,11 +1674,8 @@ static void test_SHFormatDateTimeA(void) > DWORD flags; > INT ret; > > -if (0) > -{ > - /* crashes on native */ > + if (0) /* crashes on Windows XP to Windows 10 20H2 */ > pSHFormatDateTimeA(NULL, NULL, NULL, 0); > -} > > GetLocalTime(&st); > SystemTimeToFileTime(&st, &filetime); > I don't personally care where the if (0) is, but if it's going to be there, could we maybe indent the body?

Aug. 31, 2021

Re: RFC: Reparse Point/NT Symlink Support [3]

by Erich E. Hoover

On Tue, Aug 31, 2021 at 10:24 AM Zebediah Figura (she/her) <zfigura(a)codeweavers.com> wrote: > ... > How useful are Windows symlinks, though? As far as I understand normal > users aren't allowed to create them by default, and only a few > applications even bother using them for any reason. Granted, I might not > have a particularly accurate picture of the situation. Junction Points (effectively the same from our perspective) do not require administrator privileges, so they're more common. > Many applications break if there are symlinks in parts of the path they > don't expect—in theory use of symlinks should be transparent if you're > not asking for them, but they try to be symlink-aware and screw it up in > one way or another (I can pull out bug reports if necessary). We have to > kind of weirdly half-hide Unix symlinks to Windows applications as a result. Yup, that's been super fun :) Best, Erich

Aug. 31, 2021

Re: RFC: Reparse Point/NT Symlink Support [3]

by Zebediah Figura (she/her)

On 8/31/21 11:25 AM, Erich E. Hoover wrote: > On Tue, Aug 31, 2021 at 4:14 AM Alexandre Julliard <julliard(a)winehq.org> wrote: >> >> "Erich E. Hoover" <erich.e.hoover(a)gmail.com> writes: >> ... >> We are already resolving the path element by element because of case >> insensitivity, it seems to me that reparse point resolving would fit >> right in there. All you need is to make sure the initial stat() shortcut >> fails, which you can do by creating a dangling symlink for instance. >> Though my feeling is that using a normal file would make things easier, >> say by appending some magic filename suffix. > > The difficulty isn't resolving element by element, it's doing that in > two different ways depending upon the context. The parent directory > inside a symlink works differently depending on the context, where > that's not true for files with a different case ( chdir("..") behaves > the same no matter what case the directory is ). A Linux example > (Windows works the same way): > === > ~/tmplnk$ mkdir -p test/target > ~/tmplnk$ ln -s test/target link > ~/tmplnk$ cd link/ > ~/tmplnk/link$ ls .. > target > ~/tmplnk/link$ cd .. > ~/tmplnk$ ls > link test > === > The problem I ran into is that if you treat this incorrectly then it > will cause subtle breakages all over the place. How I attempted to > solve this was to keep the "original" path around and resolve it > locally everywhere that it needs to be resolved, this approach proved > to be very messy due to the large number of ways that we use unix > paths. I believe that it's also possible to instead immediately > resolve the path and pass the "resolved" path around, but then we > would need to change the working directory handling to _not_ use the > normal path processing ( otherwise you will break > SetCurrentDirectory("..") ). I say "believe" because, to my > knowledge, the working directory handling is the only special case > that requires the "original" path. This approach might be more > viable, but I didn't get around to trying it. I mostly did this for > fun, but either way I didn't think you would be a fan of these > approaches due to the unnecessary duplication of the OS path > resolution behavior of symlinks. Wait, but by the time we do path resolution, aren't we guaranteed to have an absolute path that's free of . or ..? I thought that RtlDosPathNameToNtPathName() got rid of all of those for us.

Aug. 31, 2021

[PATCH v3 2/2] wined3d: Simplify the odd line condition in gen_yv12_read().

by Anton Baskanov

Signed-off-by: Anton Baskanov <baskanov(a)gmail.com> --- dlls/wined3d/glsl_shader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c index c760d519b30..fb809052d83 100644 --- a/dlls/wined3d/glsl_shader.c +++ b/dlls/wined3d/glsl_shader.c @@ -12624,7 +12624,7 @@ static void gen_yv12_read(struct wined3d_string_buffer *buffer, /* Read odd lines from the right side (add 0.5 to the x coordinate). Keep * in mind that each line of the chroma plane corresponds to 2 lines of the * resulting image. */ - shader_addline(buffer, " if (fract(floor(texcoord.y * size.y) * 0.25 + 1.0 / 6.0) >= 0.5)\n"); + shader_addline(buffer, " if (fract(texcoord.y * size.y * 0.25) >= 0.5)\n"); shader_addline(buffer, " texcoord.x += 0.5;\n"); /* Clamp, keep the half pixel origin in mind. */ -- 2.25.1

Aug. 31, 2021

[PATCH v3 1/2] wined3d: Fix the odd line condition in gen_yv12_read().

by Anton Baskanov

Signed-off-by: Anton Baskanov <baskanov(a)gmail.com> --- The original condition is true for every other line, which is incorrect since the chroma has only half the resolution. Fixes comb artifacts around color transitions. v2: Added a comment. v3: Split condition simplification into a separate patch. --- dlls/wined3d/glsl_shader.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c index 5ae90148dc8..c760d519b30 100644 --- a/dlls/wined3d/glsl_shader.c +++ b/dlls/wined3d/glsl_shader.c @@ -12621,8 +12621,10 @@ static void gen_yv12_read(struct wined3d_string_buffer *buffer, * Don't forget to clamp the y values in into the range, otherwise we'll * get filtering bleeding. */ - /* Read odd lines from the right side (add 0.5 to the x coordinate). */ - shader_addline(buffer, " if (fract(floor(texcoord.y * size.y) * 0.5 + 1.0 / 6.0) >= 0.5)\n"); + /* Read odd lines from the right side (add 0.5 to the x coordinate). Keep + * in mind that each line of the chroma plane corresponds to 2 lines of the + * resulting image. */ + shader_addline(buffer, " if (fract(floor(texcoord.y * size.y) * 0.25 + 1.0 / 6.0) >= 0.5)\n"); shader_addline(buffer, " texcoord.x += 0.5;\n"); /* Clamp, keep the half pixel origin in mind. */ -- 2.25.1

Aug. 31, 2021

Re: RFC: Reparse Point/NT Symlink Support [3]

by Erich E. Hoover

On Tue, Aug 31, 2021 at 4:14 AM Alexandre Julliard <julliard(a)winehq.org> wrote: > > "Erich E. Hoover" <erich.e.hoover(a)gmail.com> writes: > ... > We are already resolving the path element by element because of case > insensitivity, it seems to me that reparse point resolving would fit > right in there. All you need is to make sure the initial stat() shortcut > fails, which you can do by creating a dangling symlink for instance. > Though my feeling is that using a normal file would make things easier, > say by appending some magic filename suffix. The difficulty isn't resolving element by element, it's doing that in two different ways depending upon the context. The parent directory inside a symlink works differently depending on the context, where that's not true for files with a different case ( chdir("..") behaves the same no matter what case the directory is ). A Linux example (Windows works the same way): === ~/tmplnk$ mkdir -p test/target ~/tmplnk$ ln -s test/target link ~/tmplnk$ cd link/ ~/tmplnk/link$ ls .. target ~/tmplnk/link$ cd .. ~/tmplnk$ ls link test === The problem I ran into is that if you treat this incorrectly then it will cause subtle breakages all over the place. How I attempted to solve this was to keep the "original" path around and resolve it locally everywhere that it needs to be resolved, this approach proved to be very messy due to the large number of ways that we use unix paths. I believe that it's also possible to instead immediately resolve the path and pass the "resolved" path around, but then we would need to change the working directory handling to _not_ use the normal path processing ( otherwise you will break SetCurrentDirectory("..") ). I say "believe" because, to my knowledge, the working directory handling is the only special case that requires the "original" path. This approach might be more viable, but I didn't get around to trying it. I mostly did this for fun, but either way I didn't think you would be a fan of these approaches due to the unnecessary duplication of the OS path resolution behavior of symlinks. Best, Erich

Aug. 31, 2021

Re: RFC: Reparse Point/NT Symlink Support [3]

by Zebediah Figura (she/her)

On 8/31/21 6:16 AM, Martin Storsjö wrote: > On Sat, 28 Aug 2021, Alexandre Julliard wrote: > >> I would first question the assumption that these have to be actual, >> resolvable symlinks. What if they weren't? > > I guess one drawback is interoperability with cases where you run commands > in a mixed wine/native environment, i.e. mostly operate directly on the > filesystem with native tools, but run some subcommands that are use wine > for executing foreign binaries. (I e.g. have setups for using MSVC in this > fashion.) > > I don't have a concrete case where such tools would need to create > symlinks though, just raising it as a potential use case. > > (With the current patchset, with the extra bits encoded in the symlink > path, they do look quite weird, but if they resolve and work as regular > symlinks, that's always a plus.) How useful are Windows symlinks, though? As far as I understand normal users aren't allowed to create them by default, and only a few applications even bother using them for any reason. Granted, I might not have a particularly accurate picture of the situation. Many applications break if there are symlinks in parts of the path they don't expect—in theory use of symlinks should be transparent if you're not asking for them, but they try to be symlink-aware and screw it up in one way or another (I can pull out bug reports if necessary). We have to kind of weirdly half-hide Unix symlinks to Windows applications as a result. (Disclaimer: I'm not attached to either solution.)

Aug. 31, 2021