Hello, My name is Carlos Rivera, I would like to contribute to Wine, I think it is an incredible project. A few months ago I talked to Jeremy White, I was looking to change jobs and I saw an ad for a job offer as a codeweaver's developer. Anyway, I decided that this project inspired too much respect and I wanted to take it easy and try to collaborate first, to see If I was up to the task. During these months I haven't had much time to spend on Wine, I have read the documentation and following the advice on the web I haved looked for FIXMES in the code. I think I may have found a couple of things that hopefully I may be able to work on and give it a try. It is these two things:
file: server/change.c function: do_change_notify, line 306
/* FIXME: this is O(n) ... probably can be improved */ LIST_FOR_EACH_ENTRY( dir, &change_list, struct dir, entry ) { if (get_unix_fd( dir->fd ) != unix_fd) continue; dir->notified = 1; break; }
I think I could try to implement a hash table to avoid the loop.
Also in the same file, line 396, function dir_set_sd:
if (!obj->sd || !security_equal_sid( owner, sd_get_owner( obj->sd ) )) { /* FIXME: get Unix uid and call fchown */ }
I think I need to figure out the uid from perhaps PAM? I am not sure, but I am willing to find out if you people think this is worthwhile. I don't know how useful fixing these things may be, but they may be a good way to get started. What do you think?
Regards, Carlos.
Am 12.07.2020 um 22:29 schrieb carlos carlos@superkaos.org:
/* FIXME: this is O(n) ... probably can be improved */ LIST_FOR_EACH_ENTRY( dir, &change_list, struct dir, entry ) { if (get_unix_fd( dir->fd ) != unix_fd) continue; dir->notified = 1; break; }
I think I could try to implement a hash table to avoid the loop.
We have a red-black tree implementation that might come in handy here: see include/wine/rbtree.h
Be warned though that there's probably a reason why this hasn't changed yet - maybe we need to preserve the order somewhere, or need O(1) inserts, etc, or maybe the list is usually just a few entries long so the algorithmic complexity is less important than cache locality. Try to find an application that is affected by this, see where else change_list is used, ...
if (!obj->sd || !security_equal_sid( owner, sd_get_owner( obj->sd ) )) { /* FIXME: get Unix uid and call fchown */ }I think I need to figure out the uid from perhaps PAM? I am not sure, but I am willing to find out if you people think this is worthwhile. I don't know how useful fixing these things may be, but they may be a good way to get started. What do you think?
I am not an expert on this code, but my gut feeling is that a lot more infrastructure is needed. Wine doesn't have any multi-user capabilities on its own. The intention is that the Unix user running Wine is the sole owner of the "fake" windows installation (by default everything is in $HOME/.wine) and you can change whatever you want. Representing Unix users as Windows users inside a Wine prefix only makes sense if we aim to make multiple Unix users able to run applications in the same prefix concurrently, have them communicate with each other etc. Right now this is not our goal and I don't think users care much.
On 12/07/20 22:44, Stefan Dösinger wrote:
Am 12.07.2020 um 22:29 schrieb carlos carlos@superkaos.org:
/* FIXME: this is O(n) ... probably can be improved */ LIST_FOR_EACH_ENTRY( dir, &change_list, struct dir, entry ) { if (get_unix_fd( dir->fd ) != unix_fd) continue; dir->notified = 1; break; }
I think I could try to implement a hash table to avoid the loop.
We have a red-black tree implementation that might come in handy here: see include/wine/rbtree.h
Be warned though that there's probably a reason why this hasn't changed yet - maybe we need to preserve the order somewhere, or need O(1) inserts, etc, or maybe the list is usually just a few entries long so the algorithmic complexity is less important than cache locality. Try to find an application that is affected by this, see where else change_list is used, ...
Thanks! I'll take a look at the implementation and I'll try to find an application as you say, perhaps there is a regression test that may be affected too?
if (!obj->sd || !security_equal_sid( owner, sd_get_owner( obj->sd ) )) { /* FIXME: get Unix uid and call fchown */ }I think I need to figure out the uid from perhaps PAM? I am not sure, but I am willing to find out if you people think this is worthwhile. I don't know how useful fixing these things may be, but they may be a good way to get started. What do you think?
I am not an expert on this code, but my gut feeling is that a lot more infrastructure is needed. Wine doesn't have any multi-user capabilities on its own. The intention is that the Unix user running Wine is the sole owner of the "fake" windows installation (by default everything is in $HOME/.wine) and you can change whatever you want. Representing Unix users as Windows users inside a Wine prefix only makes sense if we aim to make multiple Unix users able to run applications in the same prefix concurrently, have them communicate with each other etc. Right now this is not our goal and I don't think users care much.
Ok, I think I saw a bug report where someone complained that an application failed because it set a file ACL permissions, then it read them back and they didn't match what was expected (because the set didn't work). I can't find it right now because winehq.org is down. But I think discussion went along those lines as you say. I will ignore this then.
crc.
Am 12.07.2020 um 23:06 schrieb carlos carlos@superkaos.org:
Thanks! I'll take a look at the implementation and I'll try to find an application as you say, perhaps there is a regression test that may be affected too?
Our regression tests currently don't care about performance. That's another big topic. I once had an automated system to test various popular games and Windows benchmarks automatically every night, but the system bitrotted and I've stopped caring about it a few years ago.
Ok, I think I saw a bug report where someone complained that an application failed because it set a file ACL permissions, then it read them back and they didn't match what was expected (because the set didn't work). I can't find it right now because winehq.org is down. But I think discussion went along those lines as you say. I will ignore this then.
That depends on the exact details. There may be a way to map the windows ACL to unix without requiring multi-user infrastructure. I've never dealt with ACLs or permissions beyond the standard unix rwxrwxrwx, but e.g. the windows read-only flag gets mapped to unix write permissions. Something similar might be possible for ACLs too.
-
carlos -
Stefan Dösinger