Re: openlog

[Matthew Wild <mwild1@...>]

On 14 August 2013 05:02, Gary V. Vaughan <gary@vaughan.pe> wrote:
> Hi Matthew,
>
> On Aug 13, 2013, at 8:29 PM, Matthew Wild <mwild1@gmail.com> wrote:
>
>> On 13 August 2013 14:13, dcharno <dcharno@comcast.net> wrote:
>>> The GNU libc manual says the pointer to ident in openlog is retained and the memory shouldn't be freed. I noticed the openlog binding in luaposix doesn't do anything special to make sure the string isn't garbage collected and neither do most FFI bindings for syslog. I haven't experienced a problem (yet) so maybe it isn't an issue. How do people handle this in their bindings.
>>
>> This is one of the reasons we're not using luaposix in Prosody yet. We
>> use this code, which works for us so far:
>> http://hg.prosody.im/trunk/file/e8c79796ead9/util-src/pposix.c#l163
>>
>> It would be nice to handle this well in luaposix.
>
> I don't personally use all the calls available in luaposix, but I am very happy to apply patches to improve the parts I don't use myself… or even write patches for those parts if someone sends me a test (any smallish self contained Lua snippet is perfect!) that will verify my implementation is correct :)

A year or two ago I evaluated luaposix, with a hope to switch Prosody
to using it instead of our own little library. Ultimately it was a
bunch of minor issues that put me off at the time. I had a small email
exchange with Reuben Thomas at the time. I'll include a relevant
section from one of my emails:

<<<<<(

Actually this morning I went through your repository. I'm really happy
to see that many of my complaints have been fixed already. So I'll
just list what I think are the open ones here (I haven't confirmed
these with code, and as far as I know there are no docs):

1) Silly API decisions

Minor on the surface, but makes the library and code using it just
seem unnecessarily ugly. One example, getpid() returns a table. I see
someone was trying to make it smart, but getpid("pid") just seems
silly. Let's see about getuid()... oh, no, that's getpid("uid"),
despite a uid not being anything like a pid. How about setting it?
setpid("u", ...). Sigh.

My approach would be to bind individual POSIX functions as tightly as
possible, and only deviating when necessary to keep the interface to
Lua clean and usable. It seems like the current API can't decide
whether it wants to be high or low level.

2) Syslog bug

Quoting from the syslog manpage:

  The argument ident in the call of openlog() is probably stored as-is.  Thus,
  if the string it points to is changed, syslog() may start prepending the
  changed string, and if the string it points to ceases to exist, the
results are
  undefined.

The current implementation just uses the return from
luaL_checkstring(), which is not guaranteed to remain after the string
leaves the stack. In my implementation I used a static variable to
hold the last ident string used (openlog is per-process anyway). Open
to other suggestions.

3) Error reporting

In Prosody's POSIX library we have fixed error strings such as
"permission-denied", which are more important than the
locale-dependent human-readable strings given by strerror() (ideally
we would have a function to return strerror() for any error name).

4) Documentation

A big issue, especially with the current bizarre API - there's
basically no way to use the library without reading the source, unless
I've missed the documentation somewhere (I do find it hard to believe
a library that has been around so long has none).

)>>>>>

All these issues could be fixed by someone with time and motivation
and no fear of breaking 7 year-old APIs :)

Happy to help where I can.

Regards,
Matthew