June 2022 - Wine-Devel - list.winehq.org

Re: [PATCH v2 4/4] http.sys: Add support for adding multiple urls to request queues.

by Zebediah Figura

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>

June 16, 2022

Re: [PATCH v2 2/4] httpapi/tests: Add tests for when URLs fail to be added to a queue.

by Zebediah Figura

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>

June 16, 2022

Re: [PATCH v2 3/4] httpapi: Stop setting group->url if add_url() fails.

by Zebediah Figura

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>

June 16, 2022

Re: [PATCH v2 1/4] httpapi/tests: Move sleep statement.

by Zebediah Figura

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>

June 16, 2022

Re: Gitlab transition roadmap

by Zebediah Figura

On 6/16/22 11:22, Gabriel Ivăncescu wrote: >> - Work with respective maintainers to plan if/when/how to move other >> Wine projects to Gitlab: >> - tools (done) >> - website (done) >> - wine-gecko (done) >> - wine-mono >> - wine-staging >> - vkd3d >> - appdb >> - bugzilla >> - fontforge >> > > For bugzilla I'd rather prefer if it remained because, well, it works > without javascript and is simple and lightweight... I don't mind web UIs > if they're simple but gitlab overall leaves a lot to be desired. Yes, I agree. Gitlab's UI in general is pretty awful, and I'd hate to have to use it for bug tracking. And while command line API tools are fine for merge requests, and the same tools can access the bug tracker, they aren't sufficient for reading structured data. > >> - Retire the gitweb browser, redirect to Gitlab. >> > > As above, is it too much hassle to keep this around? Even if it's just a > mirror. I like that it's simple and lightweight and can be browsed > easily, gitlab is way too bloated both in design and accessing it, and > doesn't work without javascript either. > > I have to admit when I first saw the gitlab experiment, I didn't expect > a total overhaul like this even for stuff not related to patches, I > thought it would be optional. > > Just my 2 cents. > Yes, agreed wholeheartedly. Can we please keep this around?

June 16, 2022

Re: Gitlab transition roadmap

by Zebediah Figura

On 6/15/22 07:08, Alexandre Julliard wrote: > - Work with respective maintainers to plan if/when/how to move other > Wine projects to Gitlab: > - tools (done) > - website (done) > - wine-gecko (done) > - wine-mono > - wine-staging > - vkd3d > - appdb > - bugzilla > - fontforge FWIW, we'd be glad to move wine-staging to winehq.org. One thing to note, though, is that we've been taking advantage of GitHub's free build service to test building Wine (we don't run the tests, just verify it builds) on Mac OS, which we'd lose if we move to winehq.org. That said, we could easily set up a script to mirror commits back to GitHub, and I think we also want to be able to set up a testbot machine to test builds for Wine itself (bug 49729). ἔρρωσθε, Zeb

June 16, 2022

Re: [PATCH 0/7] MR212: d3d11: Create dxgi resource objects for textures and buffers.

by Zebediah Figura

On 6/16/22 05:01, Henri Verbeet wrote: > On Wed, 15 Jun 2022 at 20:09, Zebediah Figura <zfigura(a)codeweavers.com> wrote: >> One thing we could probably do in general is make it possible to delay >> setting the parent ops, e.g. set it as wined3d_null_parent_ops initially >> and then add a separate function like wined3d_resource_set_parent_ops() >> once the client object is fully initialized. > > Right, that probably makes sense, although the implications for > objects currently referenced by the command stream probably deserve a > look. Hmm, I don't see what you mean? Since the only parent op is currently the destruction callback, it can't be called until all references have been dropped, and it'd definitely be an error to call wined3d_resource_set_parent_ops() without holding a reference to the resource.

June 16, 2022

Re: Gitlab experiment wrapup

by Gabriel Ivăncescu

On 16/06/2022 20:13, Alexandre Julliard wrote: > Alex Henrie <alexhenrie24(a)gmail.com> writes: > >> On Thu, Jun 16, 2022 at 10:04 AM Gabriel Ivăncescu >> <gabrielopcode(a)gmail.com> wrote: >>> >>> There's one other (pretty big, for me) problem I can't seem to find how >>> to replicate with MRs compared to sending patches: how do I add notes >>> for each commit that shouldn't actually be committed? For patches I used >>> to add below the --- line, and these are super useful when you just want >>> to tell the information to the reviewer, which wouldn't make much sense >>> to have in the codebase itself. >>> >>> These seem to get lost when I push. I have to admit `git notes` seems >>> pretty convoluted to me and I've no idea how it gets "stored" especially >>> for merge requests. Patches were much easier to comprehend. >>> >>> I mean, I guess I can add it to the MR description but that's not >>> pointing out to a specific commit/patch... sigh. >> >> In a GitLab merge request, I can click on a specific commit to see its >> diff, then click on the commit hash again, and GitLab gives me a box >> to leave a comment either about a specific line of the commit or about >> the commit as a whole. Would that work for you? > > Unfortunately comments tied to a specific commit are no longer visible > when the branch is rebased, for instance when a reviewer pushes fixups. > There's a issue filed with Gitlab about that, but until this is fixed > it's better to avoid commit-specific comments. > I might go with a normal comment (after a MR is created or after a force push for e.g. v2), with something like: **Notes for `ntdll: Foobar`** Notes go here. Multiple lines. etc. **Notes for `server: Barfoo`** v2: blah. and so on. Which can also be scripted (and easily taken from own git notes). For example with something like: git log --reverse --pretty='format:**Notes for `%s`**%n%n%N%n%n' origin..HEAD Maybe we can standardize on a common method to encourage and put in the wiki? Well, or maybe it's just me.

June 16, 2022

[PATCH v2 4/4] http.sys: Add support for adding multiple urls to request queues.

by Julian Klemann

The request queue has a growable array rather than single URL and socket properties. Signed-off-by: Julian Klemann <jklemann(a)codeweavers.com> --- v2: Fix memory leak, move iterator variables to start of function. --- dlls/http.sys/http.c | 137 ++++++++++++++++++++++++----------- dlls/http.sys/request.h | 2 +- dlls/httpapi/tests/httpapi.c | 41 ++++++++++- 3 files changed, 136 insertions(+), 44 deletions(-) diff --git a/dlls/http.sys/http.c b/dlls/http.sys/http.c index 523c70ee12c..6c71a534069 100644 --- a/dlls/http.sys/http.c +++ b/dlls/http.sys/http.c @@ -70,6 +70,7 @@ struct connection BOOL available; struct request_queue *queue; HTTP_REQUEST_ID req_id; + HTTP_URL_CONTEXT context; /* Things we already parsed out of the request header in parse_request(). * These are valid only if "available" is TRUE. */ @@ -82,15 +83,21 @@ struct connection static struct list connections = LIST_INIT(connections); -struct request_queue +struct url { struct list entry; - LIST_ENTRY irp_queue; - HTTP_URL_CONTEXT context; char *url; + HTTP_URL_CONTEXT context; SOCKET socket; }; +struct request_queue +{ + struct list entry; + LIST_ENTRY irp_queue; + struct list urls; +}; + static struct list request_queues = LIST_INIT(request_queues); static void accept_connection(SOCKET socket) @@ -319,17 +326,30 @@ static int parse_number(const char *str, const char **endptr, const char *end) return n; } -static BOOL host_matches(const struct connection *conn, const struct request_queue *queue) +static struct url *host_matches(const struct connection *conn, const struct request_queue *queue) { const char *conn_host = (conn->url[0] == '/') ? conn->host : conn->url + 7; + struct url *url; - if (queue->url[7] == '+') + LIST_FOR_EACH_ENTRY(url, &queue->urls, struct url, entry) { - const char *queue_port = strchr(queue->url + 7, ':'); - return !strncmp(queue_port, strchr(conn_host, ':'), strlen(queue_port) - 1 /* strip final slash */); + if (url->url) + { + if (url->url && url->url[7] == '+') + { + /* strip final slash */ + const char *queue_port = strchr(url->url + 7, ':'); + if (!strncmp(queue_port, strchr(conn_host, ':'), strlen(queue_port) - 1)) + return url; + } + + /* strip final slash */ + if (!memicmp(url->url + 7, conn_host, strlen(url->url) - 8)) + return url; + } } - return !memicmp(queue->url + 7, conn_host, strlen(queue->url) - 8 /* strip final slash */); + return NULL; } /* Upon receiving a request, parse it to ensure that it is a valid HTTP request, @@ -339,6 +359,7 @@ static int parse_request(struct connection *conn) { const char *const req = conn->buffer, *const end = conn->buffer + conn->len; struct request_queue *queue; + struct url *conn_url; const char *p = req, *q; int len, ret; @@ -432,10 +453,11 @@ static int parse_request(struct connection *conn) /* Find a queue which can receive this request. */ LIST_FOR_EACH_ENTRY(queue, &request_queues, struct request_queue, entry) { - if (host_matches(conn, queue)) + if ((conn_url = host_matches(conn, queue))) { TRACE("Assigning request to queue %p.\n", queue); conn->queue = queue; + conn->context = conn_url->context; break; } } @@ -544,6 +566,7 @@ static DWORD WINAPI request_thread_proc(void *arg) { struct connection *conn, *cursor; struct request_queue *queue; + struct url *url; TRACE("Starting request thread.\n"); @@ -553,8 +576,11 @@ static DWORD WINAPI request_thread_proc(void *arg) LIST_FOR_EACH_ENTRY(queue, &request_queues, struct request_queue, entry) { - if (queue->socket != -1) - accept_connection(queue->socket); + LIST_FOR_EACH_ENTRY(url, &queue->urls, struct url, entry) + { + if (url->socket != -1) + accept_connection(url->socket); + } } LIST_FOR_EACH_ENTRY_SAFE(conn, cursor, &connections, struct connection, entry) @@ -575,6 +601,7 @@ static NTSTATUS http_add_url(struct request_queue *queue, IRP *irp) const struct http_add_url_params *params = irp->AssociatedIrp.SystemBuffer; struct sockaddr_in addr; struct connection *conn; + struct url *url_entry, *new_entry; unsigned int count = 0; char *url, *endptr; ULONG true = 1; @@ -594,7 +621,7 @@ static NTSTATUS http_add_url(struct request_queue *queue, IRP *irp) if (!(addr.sin_port = htons(strtol(strchr(params->url + 7, ':') + 1, &endptr, 10))) || *endptr != '/') return STATUS_INVALID_PARAMETER; - if (!(url = heap_alloc(strlen(params->url)+1))) + if (!(url = malloc(strlen(params->url)+1))) return STATUS_NO_MEMORY; strcpy(url, params->url); @@ -603,27 +630,31 @@ static NTSTATUS http_add_url(struct request_queue *queue, IRP *irp) if (count > 3) FIXME("Binding to relative URIs is not implemented; binding to all URIs instead.\n"); - EnterCriticalSection(&http_cs); - - if (queue->url && !strcmp(queue->url, url)) + if (!(new_entry = malloc(sizeof(struct url)))) { - LeaveCriticalSection(&http_cs); - heap_free(url); - return STATUS_OBJECT_NAME_COLLISION; + free(url); + return STATUS_NO_MEMORY; } - else if (queue->url) + + EnterCriticalSection(&http_cs); + + LIST_FOR_EACH_ENTRY(url_entry, &queue->urls, struct url, entry) { - FIXME("Binding to multiple URLs is not implemented.\n"); - LeaveCriticalSection(&http_cs); - heap_free(url); - return STATUS_NOT_IMPLEMENTED; + if (url_entry->url && !strcmp(url_entry->url, url)) + { + LeaveCriticalSection(&http_cs); + free(url); + free(new_entry); + return STATUS_OBJECT_NAME_COLLISION; + } } if ((s = socket(AF_INET, SOCK_STREAM, 0)) == INVALID_SOCKET) { ERR("Failed to create socket, error %u.\n", WSAGetLastError()); LeaveCriticalSection(&http_cs); - heap_free(url); + free(url); + free(new_entry); return STATUS_UNSUCCESSFUL; } @@ -633,7 +664,8 @@ static NTSTATUS http_add_url(struct request_queue *queue, IRP *irp) { LeaveCriticalSection(&http_cs); closesocket(s); - heap_free(url); + free(url); + free(new_entry); if (WSAGetLastError() == WSAEADDRINUSE) { WARN("Address %s is already in use.\n", debugstr_a(params->url)); @@ -653,15 +685,17 @@ static NTSTATUS http_add_url(struct request_queue *queue, IRP *irp) ERR("Failed to listen to port %u, error %u.\n", addr.sin_port, WSAGetLastError()); LeaveCriticalSection(&http_cs); closesocket(s); - heap_free(url); + free(url); + free(new_entry); return STATUS_OBJECT_NAME_COLLISION; } ioctlsocket(s, FIONBIO, &true); WSAEventSelect(s, request_event, FD_ACCEPT); - queue->socket = s; - queue->url = url; - queue->context = params->context; + new_entry->url = url; + new_entry->context = params->context; + new_entry->socket = s; + list_add_head(&queue->urls, &new_entry->entry); /* See if any pending requests now match this queue. */ LIST_FOR_EACH_ENTRY(conn, &connections, struct connection, entry) @@ -669,6 +703,7 @@ static NTSTATUS http_add_url(struct request_queue *queue, IRP *irp) if (conn->available && !conn->queue && host_matches(conn, queue)) { conn->queue = queue; + conn->context = params->context; try_complete_irp(conn); } } @@ -681,21 +716,33 @@ static NTSTATUS http_add_url(struct request_queue *queue, IRP *irp) static NTSTATUS http_remove_url(struct request_queue *queue, IRP *irp) { const char *url = irp->AssociatedIrp.SystemBuffer; + struct url *url_entry; TRACE("host %s.\n", debugstr_a(url)); EnterCriticalSection(&http_cs); - if (!queue->url || strcmp(url, queue->url)) + LIST_FOR_EACH_ENTRY(url_entry, &queue->urls, struct url, entry) { - LeaveCriticalSection(&http_cs); - return STATUS_OBJECT_NAME_NOT_FOUND; + if (url_entry->url && !strcmp(url, url_entry->url)) + { + free(url_entry->url); + url_entry->url = NULL; + + shutdown(url_entry->socket, SD_BOTH); + closesocket(url_entry->socket); + url_entry->socket = -1; + + list_remove(&url_entry->entry); + free(url_entry); + + LeaveCriticalSection(&http_cs); + return STATUS_SUCCESS; + } } - heap_free(queue->url); - queue->url = NULL; LeaveCriticalSection(&http_cs); - return STATUS_SUCCESS; + return STATUS_OBJECT_NAME_NOT_FOUND; } static struct connection *get_connection(HTTP_REQUEST_ID req_id) @@ -892,8 +939,9 @@ static NTSTATUS WINAPI dispatch_create(DEVICE_OBJECT *device, IRP *irp) IO_STACK_LOCATION *stack = IoGetCurrentIrpStackLocation(irp); struct request_queue *queue; - if (!(queue = heap_alloc_zero(sizeof(*queue)))) + if (!(queue = calloc(1, sizeof(*queue)))) return STATUS_NO_MEMORY; + list_init(&queue->urls); stack->FileObject->FsContext = queue; InitializeListHead(&queue->irp_queue); @@ -910,17 +958,22 @@ static NTSTATUS WINAPI dispatch_create(DEVICE_OBJECT *device, IRP *irp) static void close_queue(struct request_queue *queue) { + struct url *url, *url_next; + EnterCriticalSection(&http_cs); list_remove(&queue->entry); - if (queue->socket != -1) + + LIST_FOR_EACH_ENTRY_SAFE(url, url_next, &queue->urls, struct url, entry) { - shutdown(queue->socket, SD_BOTH); - closesocket(queue->socket); + free(url->url); + shutdown(url->socket, SD_BOTH); + closesocket(url->socket); + list_remove(&url->entry); + free(url); } - LeaveCriticalSection(&http_cs); + free(queue); - heap_free(queue->url); - heap_free(queue); + LeaveCriticalSection(&http_cs); } static NTSTATUS WINAPI dispatch_close(DEVICE_OBJECT *device, IRP *irp) diff --git a/dlls/http.sys/request.h b/dlls/http.sys/request.h index f044db3bb2c..f307ca3dead 100644 --- a/dlls/http.sys/request.h +++ b/dlls/http.sys/request.h @@ -181,9 +181,9 @@ static NTSTATUS complete_irp(struct connection *conn, IRP *irp) offset = sizeof(*req); + req->UrlContext = conn->context; req->ConnectionId = (ULONG_PTR)conn; req->RequestId = conn->req_id; - req->UrlContext = conn->queue->context; req->Version = conn->version; req->Verb = conn->verb; req->UnknownVerbLength = conn->unk_verb_len; diff --git a/dlls/httpapi/tests/httpapi.c b/dlls/httpapi/tests/httpapi.c index 3c3f7abcd90..be7cb667cea 100644 --- a/dlls/httpapi/tests/httpapi.c +++ b/dlls/httpapi/tests/httpapi.c @@ -108,7 +108,7 @@ static unsigned short add_url_v1(HANDLE queue) swprintf(url, ARRAY_SIZE(url), L"http://localhost:%u/", port); if (!(ret = HttpAddUrl(queue, url, NULL))) return port; - ok(ret == ERROR_SHARING_VIOLATION, "Failed to add %s, error %u.\n", debugstr_w(url), ret); + ok(ret == ERROR_SHARING_VIOLATION || ret == ERROR_ALREADY_EXISTS, "Failed to add %s, error %u.\n", debugstr_w(url), ret); } ok(0, "Failed to add url %s, error %u.\n", debugstr_w(url), ret); return 0; @@ -1083,6 +1083,44 @@ static void test_v1_unknown_tokens(void) ok(ret, "Failed to close queue handle, error %lu.\n", GetLastError()); } +static void test_v1_multiple_urls(void) +{ + char DECLSPEC_ALIGN(8) req_buffer[2048]; + HTTP_REQUEST_V1 *req = (HTTP_REQUEST_V1 *)req_buffer; + unsigned short ports[4]; + char req_text[200]; + DWORD ret_size; + HANDLE queue; + SOCKET s; + unsigned int i; + int ret; + + ret = HttpCreateHttpHandle(&queue, 0); + ok(!ret, "Got error %u.\n", ret); + + for (i = 0; i < 4; ++i) + ports[i] = add_url_v1(queue); + + for (i = 0; i < 4; ++i) + { + s = create_client_socket(ports[i]); + sprintf(req_text, simple_req, ports[i]); + ret = send(s, req_text, strlen(req_text), 0); + ok(ret == strlen(req_text), "send() returned %d.\n", ret); + + memset(req_buffer, 0xcc, sizeof(req_buffer)); + ret = HttpReceiveHttpRequest(queue, HTTP_NULL_ID, 0, (HTTP_REQUEST *)req, sizeof(req_buffer), &ret_size, NULL); + ok(!ret, "Got error %u.\n", ret); + ok(ret_size > sizeof(*req), "Got size %lu.\n", ret_size); + + send_response_v1(queue, req->RequestId, s); + closesocket(s); + } + + ret = CloseHandle(queue); + ok(ret, "Failed to close queue handle, error %lu.\n", GetLastError()); +} + static void test_v1_urls(void) { char DECLSPEC_ALIGN(8) req_buffer[2048]; @@ -1643,6 +1681,7 @@ START_TEST(httpapi) test_v1_bad_request(); test_v1_cooked_url(); test_v1_unknown_tokens(); + test_v1_multiple_urls(); test_v1_urls(); ret = HttpTerminate(HTTP_INITIALIZE_SERVER, NULL); -- 2.36.1

June 16, 2022

[PATCH v2 3/4] httpapi: Stop setting group->url if add_url() fails.

by Julian Klemann

The URL should not be set if the URL cannot be added to a request queue. Signed-off-by: Julian Klemann <jklemann(a)codeweavers.com> --- v2: No change. --- dlls/httpapi/httpapi_main.c | 11 ++++++++--- dlls/httpapi/tests/httpapi.c | 4 ++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/dlls/httpapi/httpapi_main.c b/dlls/httpapi/httpapi_main.c index 4d2d307ffcb..a5f8007b0a9 100644 --- a/dlls/httpapi/httpapi_main.c +++ b/dlls/httpapi/httpapi_main.c @@ -686,6 +686,7 @@ ULONG WINAPI HttpAddUrlToUrlGroup(HTTP_URL_GROUP_ID id, const WCHAR *url, HTTP_URL_CONTEXT context, ULONG reserved) { struct url_group *group = get_url_group(id); + ULONG ret; TRACE("id %s, url %s, context %s, reserved %#lx.\n", wine_dbgstr_longlong(id), debugstr_w(url), wine_dbgstr_longlong(context), reserved); @@ -696,13 +697,17 @@ ULONG WINAPI HttpAddUrlToUrlGroup(HTTP_URL_GROUP_ID id, const WCHAR *url, return ERROR_CALL_NOT_IMPLEMENTED; } + if (group->queue) + { + ret = add_url(group->queue, url, context); + if (ret) + return ret; + } + if (!(group->url = heap_strdupW(url))) return ERROR_OUTOFMEMORY; group->context = context; - if (group->queue) - return add_url(group->queue, url, context); - return ERROR_SUCCESS; } diff --git a/dlls/httpapi/tests/httpapi.c b/dlls/httpapi/tests/httpapi.c index d231fd1d8ed..3c3f7abcd90 100644 --- a/dlls/httpapi/tests/httpapi.c +++ b/dlls/httpapi/tests/httpapi.c @@ -1268,8 +1268,8 @@ static void test_v2_bound_port(void) s = socket(AF_INET, SOCK_STREAM, 0); ret = connect(s, (struct sockaddr *)&sockaddr, sizeof(sockaddr)); - todo_wine ok(ret, "Connecting to socket succeeded, %lu.\n", GetLastError()); - todo_wine ok(GetLastError() == WSAECONNREFUSED, "Unexpected error connecting to socket, %lu.\n", GetLastError()); + ok(ret, "Connecting to socket succeeded, %lu.\n", GetLastError()); + ok(GetLastError() == WSAECONNREFUSED, "Unexpected error connecting to socket, %lu.\n", GetLastError()); closesocket(s); ret = pHttpCloseRequestQueue(dummy_queue); -- 2.36.1

June 16, 2022