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

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

From: Pavel Strashkin <pavel.strashkin_at_gmail.com>
Date: Mon, 17 Oct 2011 20:40:39 -0700

> Please check and answer this?

It was sarcasm. No one of those constants are documented. If something
is done - i did it too. If not - i'm not too.

> 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.

OK, now it sounds better. The last question is how we should check
that passed signal name is valid? I have "valid_signals" array. Keep
current code as is?
btw. i saw many peaces of code related to the signals and i didn't see
code-owns-definition of signals. I mean, we already have POSIX and if
you're not going to respect it and pass wrong value - it's your
problem. Either way is good for me. If you'd like to see defines i can
add it. It isn't the point where i'd to stop and discuss it forever.
Code is done and ready for use. Everything else is up to commiters.

> 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.

You know, it's not a big deal to write join-code. It's being called
everytime when libssh2_channel_signal happens. Another solution is
just remove advice from error message, but i don't like this idea. If
something goes wrong - i don't want to check doc or code. Error
message should be self-describable and as much helpful and it can be.

2011/10/17 Peter Stuge <peter_at_stuge.se>:
> 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
>
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2011-10-18