I've started working on the changes for the mh installation thing discussed
earlier. My plan is to modify context_read() to print a message instead of
invoking install-mh, and to add a -check option to install-mh that allows it
to silently check for installation, returning the status via the exit code.
There's a bunch of strange stuff in context_read() that seems unnecessary.
I'd like to get rid of it, but want to check with y'all in case there's some
undocumented historic necessity of which I'm unaware. I know that these are
picky things, but I'm just not in the write-sloppy-and-slow-software-camp even
though computers are pretty darned fast these days.
1. The program starts by checking to see whether or not defpath has been
set, and bails if it has. Apparently a test in case it's called more
than once. I've checked all of the other programs, and it never is.
defpath is unnecessarily checked in a number of other places:
sbr/context_del.c
sbr/context_find.c
sbr/context_replace.c
sbr/seq_read.c
sbr/seq_save.c
uip/flist.c
uip/folder.c
uip/rmf.c
I'd prefer to see asserts used to check for dumb programming errors.
2. Next there's a test to see if mypath is set, and setting it is skipped
if it's already set. Again, I don't see any way that this could occur.
3. If the $HOME environment variable is set, mypath is copied from the
getenv return. Why? It's never changed.
4. If the $HOME environment variable is not set, mypath is copied from the
pw_dir member of the returned passwd structure. Now, I understand that
this is a static structure, but getpwuid is never called again so I
don't see why the copy is needed.
4. If the $HOME environment variable is not set, the pw_dir member of the
passwd structure returned by getpwuid() is checked for a NULL pointer.
This can never happen in a non-error return, which is already checked.
So why the superfluous check?
5. There's code that removes a trailing / from mypath if mypath is more
than one character long. Seems completely unnecessary; the definition
of path names means that // is interpreted as /. And if that wasn't
the case, this code wouldn't catch multiple trailing slashes anyway.
6. If an MH environment variable is set and doesn't begin with a / the
MH environment variable is changed to the profile path. Why bother?
It's never used by anything. Exec'd programs can get figure this out
if they need to - any exec'd nmh program does that anyway.
Jon
