[OPEN-ILS-DEV] PATCH: osrf_system.c

[Mike Rylander]

On 5/20/07, Scott McKellar <mck9 at swbell.net> wrote:
> Your revision looks pretty good to me, except that you forgot to
> change the logged messages for missing usernames and passwords.
> They're still complaining about domains -- an obvious copy-and-modify
> without the modify.

Yep.  Fixed, thanks.

>
> I like the way your rearrangement lets you consolidate the frees.
> I might have suggested something similar but I didn't know which
> fields should be treated as required.
>
> If more than one required field is missing, you report all of them,
> instead of bailing out on the first one.  That's the right thing
> to do.
>
> I also note that you eliminated a strdup() of the domain.
>

Right.  Seemed unneeded to me after looking further.  The functions
that use the data strdup what they need.

> If the config file doesn't specify a log file, you're still
> returning a -1 flavor of "success" without actually connecting.  I
> don't know if that's because that's really how it should work or if
> you don't want to address that issue in the same patch.
>

Exactly.  Bill (original author of this code) is out of town for a
week, and I'm focusing on bug-fix patches and stabilizing for the
forthcoming 1.2 release.  As mentioned in a parallel thread, the -1
status may be intended to become a non-error, so I opted to leave that
alone for now.

> I intend to keep working on the configuration/logon stuff.  It isn't
> very sexy but it's important that it be robust.  Most users don't
> configure things often enough to really know what they're doing.
> When something goes wrong -- as it inevitably will -- the system
> should say exactly what the problem is, so far as possible.  Coredumps
> and segfaults leave the victims feeling helpless as well as
> frustrated.
>
> Here's a teaser for you.  If the configuration file contains extra
> text between elements -- i.e. not within tag delimiters -- we get a
> segfault when we try to translate the XML into JSON objects.  I
> pretty much know why it's happening but I haven't figured out how
> to fix it yet.
>
> One thing I don't know is: should the extra text be reported as a
> fatal error, logged in a warning message, or silently ignored?  Are
> there cases where such extra text is legitimate?
>

I'm not certain I follow.  Do you mean extra text nodes in the DOM
tree that show up as strings of whitespace between, say, the <config>
element and the <srfsh> element?  If so, I can't think of any reason
that those should be considered meaningful, and should be ignored.

Thanks, Scott

--miker

> Scott McKellar
> http://home.swbell.net/mck9/aargh/
>
> --- Mike Rylander <mrylander at gmail.com> wrote:
>
> > Scott,
> >
> > I've gathered up some of your surrounding commentary on osrf_system.c
> > (username potentially causing segfault, etc) and extended this patch
> > to include checks for other required fields.  I applied some
> > reordering, so that the log file is checked first (so we can do some
> > logging) but generally followed your convention.  If you have a
> > minute, please take a look at
> >
> >
> http://open-ils.org/cgi-bin/viewcvs.cgi/ILS/OpenSRF/src/libstack/osrf_system.c?r1=1.33&r2=1.31&diff_format=l
> >   or
> > http://ln-s.net/XWF
> >
> > and eyeball the diff for anything obvious.
> >
> > Thanks a ton for the patch ... keep 'em coming!
> >
> > --miker
>
>
>


-- 
Mike Rylander