| Re: [hatari-devel] Network support for Hatari |
[ Thread Index |
Date Index
| More lists.tuxfamily.org/hatari-devel Archives
]
Hi,
On 12.5.2021 16.07, Thorsten Otto wrote:
On Dienstag, 11. Mai 2021 23:44:23 CEST Eero Tamminen wrote:
I guess about only thing that could be done at
restore would be to close the socket.
I think that won't work, because after restoring a snapshot, the (still
running) StinG driver would then try to access the now closed sockets, and
fail.
The clean way is IMHO to store the state of the handlers (whether they are
active or not), and restart them if needed. But that would require changes to
the snapshots, and also needs some more functions to be exported.
Could you add some TODO comments about that stuff
to the patch?
one option would be to use
OS specific threading primitives directly.
I'll have a look into that later, since it looks like it has also to be fixed
in ARAnyM. I'm not so worried about a few bytes of memory being leaked, but to
my understanding the handler structure is freed, while the running thread
still accesses it.
Using os primitives does not work i think, at least i found no clean way to
get at the internal handles that would be needed (pthread_t or whatever).
I meant doing *all* the thread operations using OS
native primitives instead of SDL2 ones, not just
thread killing.
Normally you set a flag that is regularly checked in the thread, but i doubt
that this would work either in this case, since the thread is blocked in the
recv() call.
I agree. And sending process-wide signal interrupting syscalls would be
nasty as I'm not
sure whether all other parts of Hatari would react
appropriately for that (e.g. loop on all syscalls
that could fail with EINTR, such as close()).
Similarly to Nicolas, I think it would be better
if these files were in their own directory.
Done now.
Only single EthernetHandler
implementation can be compiled in at the same
time, so ETHERNET_HANDLER_CLASSNAME, different
names for the implementations in OS specific
files, and separate headers for those are all
redundant.
Yes, those separate header files are leftovers of the C++ implementation in
ARAnyM, which also declared the subclasses there. Those are now private to the
implementation, and i've combined them to a single file.
Tracing output is very inconsistent, see:
Also leftovers from ARAnyM debug output ;) Should all be prefixed by Ethernet:
now, except for the internal debug functions that dump whole frames
Thanks, on quick check these look OK now, except
for this trace entry still missing newline:
------------------------------------
+ LOG_TRACE(TRACE_ETHERNET, "Ethernet:
WinTap: Error opening registry key: %s\\%s\\%s",
+
NETWORK_CONNECTIONS_KEY, connection_string, name_string);
------------------------------------
I would also like this huge patch to be split
up to few smaller commits,
That does not make sense IMHO, since then you have intermediate commits that
either don't compile or don't work. It's a new feature after all, and not
*that* large. And most of it are new files, not changes to existing ones.
It can be split at least to two commits, first
adding the new files, and second adding the
changes to existing files to integrate the
new functionality to Hatari.
But I can do that. It would help if you'll
send the patch in "git format-patch" format
though.
- Eero