Subject: Re: [PATCH] Add support for "signal" message channel request

Re: [PATCH] Add support for "signal" message channel request

From: Peter Stuge <peter_at_stuge.se>
Date: Tue, 18 Oct 2011 05:23:05 +0200

Pavel Strashkin wrote:
> > Is this new constant documented somewhere too?
>
> Are those all constants documented somewhere? :)

Please check and answer this?

> > I disagree with this interface. First I would remove signame_len,
> > noone wants to deal with that. Second, I would rather use a uint8_t
> > or uint16_t parameter for the signal, and define all posible signals
> > in an enum in the header file. Again, noone wants to deal with
> > strings.
>
> I'd like to keep API in the same way as RFC describes it. I don't
> know why they choose strings for signals instead of numbers. There
> was the reason. They define concrete signal names. And also
> libssh2_channel_signal(..., "QUIT") looks better and more readable
> than libssh2_channel_signal(..., 7). I believe 99% programmers
> remember only than 9 == TERM and 15 == KILL.

I am not advocating using numbers in the API, but to have values in
an enum in libssh2.h, e.g. LIBSSH2_SIGNAL_QUIT. The names should
match those in the RFC of course.

> > But more importantly, I don't like that the list of these signals
> > appears in two places. I would like the error message to somehow
> > cleverly use the canonical list of signals that the code also uses.
>
> There is no easy way to join all valid_signals elements. The price
> of this duplicate is really low.

It's not about saving bytes, it's about making sure that there exists
only one canonical representation of the values, so that it is
impossible to create an inconsistency.

//Peter
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2011-10-18