Re: [Nmh-workers] miscellaneous proposed changes along the way

[Peter Maydell]

Jon Steinhart wrote:
> o  Can we just use exit() instead of done()?

Yep, if we change the way inc.c mhbuild.c mhlist.c mhlsbr.c
mhn.c mhshow.c mhstore.c mhtest.c packf.c pick.c rcvdist.c rcvstore.c
sendsbr.c currently override 'done()' and make them use atexit
functions instead. [List generated with 
egrep -r '#define\s*done' .
egrep -r 'done\s*=' .
]

I think the current done() stuff is pretty ugly and I'd be happy
to see it go.

> o  Can we just use strcpy() instead of getcpy()?

No, the semantics are different; getcpy() is much closer to strdup() if
you're looking for a replacement [but it has the wrinkle that it handles
NULL by returning a newly allocated "" string, which we presumably rely
on].

> o  Can we just use fprintf() instead of advise() adios(),
> admonish(), and advertise()?

The wrappers do buy us something: consistent formatted error messages
including things like the program name. You need advertise() because it
takes a va_list, and it's useful to have convenient ways of saying
"print message and continue" (advise) and "print message and die" (adios).
However I don't think the code makes a consistent distinction between
advise and admonish so we could lose 'admonish', I think.

I would shed no tears at all if we dropped the gratuitous implementation
of advertise() in terms of writev in favour of the straightforward
fprintf based code in the #else clause.

> o  Can we just use atoi() instead of m_atoi()?

Again, the semantics are different. m_atoi() insists that all its string
is a number, not just the initial portion. This is (implicitly) what is
implementing the restriction in command line parsing etc that a message
number has to be all digits (ie "show 34xyzzy" doesn't act like
"show 34"), I think.

However, since m_atoi()'s comments say it is to "parse a string
representation of a message number" then we should give it a name that
indicates this rather than implying that it's a general purpose atoi
replacement.

I also notice that if LOCALE is defined we use isdigit(), presumably
for the benefit of EBCDIC systems :-)

> o  Can we just use strcpy() instead of copy()?

We seem to use copy() just five times in the whole codebase (according to
grep -r '[^a-z_]copy\s*(' .
) so it's certainly a strong candidate for removal.

However note that the semantics are different (return value is end of dest
string, not start) so you'd have to fiddle with the code in the callers. I
imagine somebody wanted to avoid reading through the string all over again
to add something to the end of it.

> o  Can we get rid of the nmh versions of strcasecmp(), strncasecmp()

Yes, if you fix all the callers assuming they can pass NULL, as per the
comment in strcasecmp.c.

>, and strdup()

No, because our implementation exits if the memory allocation fails rather
than returning NULL (and we want this behaviour, it makes it much easier
to write code which uses it). However we ought to rename it mh_xstrdup() 
by analogy with mh_xmalloc() and mh_xrealloc().

-- PMM