Re: OAF patch

[Darin Adler <darin bentspoon com>]

On Monday, July 16, 2001, at 09:32  AM, Havoc Pennington wrote:

> With this patch OAF finally does report errors such as "oafd not
> found" if oaf_activate_from_id() fails. Also, it doesn't start oafd if
> you pass EXISTING_ONLY, which keeps postinstalls that e.g. run
> gconftool to install schemas from spawning off oafds.

Seems like a great idea to me. Not sure if Maciej is going to review the 
patch, so I will.

> +        if (data->iorbuf[0] == '\0' &&
> +            (condition & G_IO_IN)) {
> +                if (!fgets (data->iorbuf, sizeof (data->iorbuf), 
> data->fh)) {
> +                        g_snprintf (data->iorbuf, 128,
> +                                    _("Failed to read from child process:
>  %s\n"),
> +                                    strerror (errno));
> +
> +                        retval = FALSE;
> +                } else {
> +                        retval = TRUE;
> +                }
> +        } else {
> +                retval = FALSE;
> +        }

Where does the 128 come from?

> @@ -168,7 +183,8 @@ oaf_server_by_forking (const char **cmd,
>  				g_snprintf (cbuf, sizeof (cbuf),
>  					    _("Unknown non-exit error (status is %u)"),
>  					    status);
> -			errval->description = CORBA_string_dup (cbuf);
> +
> +                        errval->description = CORBA_string_dup (cbuf);
>  			CORBA_exception_set (ev, CORBA_USER_EXCEPTION,
>  					     ex_OAF_GeneralError, errval);
>  			return CORBA_OBJECT_NIL;

Eliminate this gratuitous change?

Overall, this looks great and I think you should commit to both branches 
if possible.

    -- Darin