On Thu, Jun 06, 2002 at 10:42:38PM +0200, Dennis Haney wrote:
>
> This is primarily a documentation patch, but it also adds a single
> error check (assert if unref is called with refcnt==0) and many
> g_warning with a exit(-1) right after was changed to g_critical. The
> states a daemon was running in was replaced by an enum to make it more
> clean what is what.
Mostly OK. I've commented on some details below.
- Dick
> +
> +The mono_handle_d is a process which takes care of the (de)allocation
> +of scratch shared memory and filehandles and refcounts of the
> +filehandles. It is designed to be run by a user and to be fast, thus
Not just file handles, there are many sorts. Please change this wording
throughout the doc and the comments.
> +
> +=head2 How to start the deamon
Please make sure you've spelled "daemon" correctly throughout (including in
typedefs :) )
> +
> +Deallocate a shared memory area, this must have been allocated before
> +deallocating. A L<WapiHandleResponse> with
> +.type=WapiHandleResponseType_Scratch Freewill be sent back (works just
Some typos here
> Index: daemon-messages.c
> @@ -71,7 +71,11 @@
> int ret;
>
> ret=recv (fd, req, sizeof(WapiHandleRequest), MSG_NOSIGNAL);
> + //we cant check ret != sizeof(WapiHandleRequest) because its a
> + //union
Please replace the // comments with /* */
>
> ret=send (fd, resp, sizeof(WapiHandleResponse), MSG_NOSIGNAL);
> - if(ret==-1) {
> #ifdef DEBUG
> + if(ret==-1 || ret != sizeof(WapiHandleResponse)) {
I don't understand this bit. Earlier you said you can't use sizeof on a
union, but here you are doing it.
> Index: daemon-messages.h
> @@ -18,6 +18,7 @@
> WapiHandleRequestType_Close,
> WapiHandleRequestType_Scratch,
> WapiHandleRequestType_ScratchFree,
> + WapiHandleRequestType_Error,
> } WapiHandleRequestType;
Please move the Error enum to the top of the list, to be symmetrical with
the responses.
> Index: daemon.c
> ===================================================================
>
> +// Array to hold the filehandles we are currently polling
> static struct pollfd *pollfds=NULL;
Comments again...
> -static gpointer *handle_refs=NULL;
> +static guint32 **handle_refs=NULL;
Good catch.
> static gboolean unref_handle (guint32 idx, guint32 handle)
> {
> guint32 *open_handles=handle_refs[idx];
> @@ -104,6 +137,8 @@
> return(FALSE);
> }
>
> + g_assert(open_handles[handle] != 0);
> +
Shouldn't use assert in the daemon. Just g_warning and ignore the request if
the handle isn't open.
> @@ -325,9 +433,19 @@
> case WapiHandleRequestType_ScratchFree:
> process_scratch_free (idx, req.u.scratch_free.idx);
> break;
> + case WapiHandleRequestType_Error: /* Falltrough */
> + default:
> + /*FIXME: call rem_fd?*/
> + break;
> }
I prefer to not use default in switch statements like this, so the compiler
will remind me when I've forgotten to add a case.
The rest is fine though.
_______________________________________________
Mono-list maillist - [EMAIL PROTECTED]
http://lists.ximian.com/mailman/listinfo/mono-list
