Francisco Casas (@fcasas) commented about libs/vkd3d-shader/hlsl_constant_ops.c:
> + {
> + switch (type)
> + {
> + case HLSL_TYPE_FLOAT:
> + case HLSL_TYPE_HALF:
> + dst->value.u[k].f = fabsf(src->value.u[k].f);
> + break;
> +
> + case HLSL_TYPE_DOUBLE:
> + dst->value.u[k].d = fabs(src->value.u[k].d);
> + break;
> +
> + case HLSL_TYPE_INT:
> + /* abs(INT_MIN) is undefined, leave this expression alone */
> + if (src->value.u[k].i == INT_MIN)
> + return false;
This won't get the absolute value of components that come after the component that is INT_MIN.
Consider an int vector such as `(-1, INT_MIN, -1, -1)`; This would end up as `(1, INT_MIN, -1, -1)`.
I think it would be better to guard the assignment:
```c
case HLSL_TYPE_INT:
/* abs(INT_MIN) is undefined, leave this expression alone */
if (src->value.u[k].i != INT_MIN)
dst->value.u[k].i = abs(src->value.u[k].i);
break;
```
--
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/203#note_33133
Adds the tray icons implementation based on org.kde.StatusNotifierItem interface usage. Does allow restarting StatusNotifierWatcher object, but will fallback to XEMBED or internal tray, if wine gets initialized when there is no StatusNotifierWatcher object registered.
--
v10: test host_os echoing
https://gitlab.winehq.org/wine/wine/-/merge_requests/2808
Adds the tray icons implementation based on org.kde.StatusNotifierItem interface usage. Does allow restarting StatusNotifierWatcher object, but will fallback to XEMBED or internal tray, if wine gets initialized when there is no StatusNotifierWatcher object registered.
--
v9: configure: fix winesni.drv not getting disabled
https://gitlab.winehq.org/wine/wine/-/merge_requests/2808
Here are a few more high-level comments I have:
* I'd like to see more tests for how paths are translated, in particular I'd like to see tests for a DOS-style absolute path name (beginning with a drive letter) and an NT-style absolute path name [i.e. whatever RtlDosPathNameToNtPathName() returns for the same path], and a path containing a single or double dot. I'd also like to see the result of getsockname() on all tested paths.
* I'd also like to see tests for sendto/recvfrom, if for no other reason than to make sure that the address translation is being done there.
* What protocol do these sockets actually have? I.e. what does SO_PROTOCOL_INFO return for them? 0 is usually a wildcard.
* I'm willing to assume that most socket functions probably work the same way (I honestly suspect that's not actually the case, but it's better just to leave that alone for now). That said, tests for any functions that your program uses and depends on would be good, lest we find out later that they're supposed to be implemented differently and break things.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_33118
Zebediah Figura (@zfigura) commented about server/fd.c:
> fd->unix_fd = open( name, O_RDONLY | (flags & ~(O_TRUNC | O_CREAT | O_EXCL)), *mode );
> }
>
> + /* open(3) throws ENXIO when `path` is one of:
> + * 1. a FIFO, and open(3) is called with O_WRONLY | O_NONBLOCK,
> + * and no other process is currently attempting to read from `path`.
> + * 2. a special device file, and the device it corresponds to does not exist.
> + * 3. a UNIX socket.
> + * We can confirm the third case to allow for unlinking a socket.
> + */
> + if (errno == ENXIO && !stat( name, &st ) && S_ISSOCK(st.st_mode))
> + {
> + if ((access & DELETE))
> + unlink( name );
This should be checked; I don't think we can depend on errno being cleared.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_33117
Zebediah Figura (@zfigura) commented about server/fd.c:
> }
>
> + /* open(3) throws ENXIO when `path` is one of:
> + * 1. a FIFO, and open(3) is called with O_WRONLY | O_NONBLOCK,
> + * and no other process is currently attempting to read from `path`.
> + * 2. a special device file, and the device it corresponds to does not exist.
> + * 3. a UNIX socket.
> + * We can confirm the third case to allow for unlinking a socket.
> + */
> + if (errno == ENXIO && !stat( name, &st ) && S_ISSOCK(st.st_mode))
> + {
> + if ((access & DELETE))
> + unlink( name );
> + else
> + {
> + set_error( ENXIO );
That's... not really correct, ENXIO isn't an NTSTATUS. Better just to let this hit the same file_set_error() path.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_33116
Zebediah Figura (@zfigura) commented about dlls/ws2_32/tests/sock.c:
> +{
> + int sockfd;
> + SOCKADDR_UN addr;
> +};
> +
> +static DWORD WINAPI test_afunix_client_connect_thread(void *param)
> +{
> + struct afunix_thread_param *in = param;
> + ok(!connect(in->sockfd, (SOCKADDR *)&in->addr, sizeof(in->addr)), "Could not connect to Unix socket: %lu\n",
> + GetLastError());
> + return 0;
> +}
> +
> +static void test_afunix(void)
> +{
> + SOCKET server, client, conn;
This naming is understandable, but a bit confusing, since much of the rest of the file uses the convention "listener, server, client", where "server" is returned from accept(). For the sake of clarity to someone reading the test I'd request that it be changed to match that.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_33115
Zebediah Figura (@zfigura) commented about dlls/ws2_32/tests/sock.c:
> + ok(ret == sizeof(serverMsg),
> + ret >= 0
> + ? "Incorrect amount of bytes written to Unix socket: %lu\n"
> + : "Could not send data over Unix socket: %lu\n",
> + ret >= 0 ? ret : GetLastError());
> +
> + ret = recv(client, clientBuf, sizeof(serverMsg), 0);
> + ok(ret == sizeof(serverMsg),
> + ret >= 0
> + ? "Incorrect amount of bytes read from Unix socket: %lu\n"
> + : "Could not receive data over Unix socket: %lu\n",
> + ret >= 0 ? ret : GetLastError());
> +
> + ok(!memcmp(serverMsg, clientBuf, sizeof(serverMsg)), "Data mismatch over Unix socket\n");
> +
> + DeleteFileA("test_afunix.sock");
This isn't very interesting if you don't test the return value.
Judging from patch 4/8, I'm guessing real software does this. "Analogous to unbinding" is an interesting statement, though. Do real world programs expect the socket to be able to be bound to a different address afterwards? We'll need tests for that, and I don't think it's currently possible in Wine.
Note that this can't possibly be a substitute for actually closing the socket handle, which this test should do for all of its sockets regardless.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_33114