* remove changes for "_libssh2_error"
* fix "memeory" typo
* add missing signals to "valid_signals" array
* use ARRAY_SIZE instead of sizeof to get number of elements in
"valid_signals" array
* make "valid_signals" array to be super constant
* fix comment header for "libssh2_channel_signal" function
> Since this list is search linearly, you could make it ever so slightly faster
> by sorting it in decreasing order or usage frequency. That would be hard to
> determine, but I'd put signal like QUIT and HUP before ones like FPE. It's a
> microoptimization, but it comes for free :)
I'd prefer to keep it in the same order as in RFC, doc file and error
string, if you don't mind.
2011/10/17 Dan Fandrich <dan_at_coneharvesters.com>:
> On Mon, Oct 17, 2011 at 01:01:03PM -0700, Pavel Strashkin wrote:
>> + * _libssh2_channel_signal
>> + *
>> + * Deliver a signal to a shell/program/subsystem.
>> + */
>> +static int _libssh2_channel_signal(LIBSSH2_CHANNEL *channel,
>> + const char *signame, size_t signame_len)
>> +{
>> + LIBSSH2_SESSION *session = channel->session;
>> + static const char *valid_signals[] = {
>
> This should be static const char * const valid_signals[] to make the array
> itself constant.
>
>> + "ABRT",
>> + "ALRM",
>> + "FPE",
>> + "HUP",
>> + "ILL",
>> + "INT",
>> + "KILL",
>> + "PIPE",
>> + "QUIT",
>> + "SEGV",
>
> Since this list is search linearly, you could make it ever so slightly faster
> by sorting it in decreasing order or usage frequency. That would be hard to
> determine, but I'd put signal like QUIT and HUP before ones like FPE. It's a
> microoptimization, but it comes for free :)
>
>> + 0
>> + };
>> +
>> + int signal_is_valid = 0;
>> + int i = 0;
>> +
>> + for (i = 0; i < sizeof(valid_signals); i++) {
>
> sizeof is wrong here. It should be ARRAY_SIZE(valid_signals).
>
>> + if (strcmp(signame, valid_signals[i]) == 0) {
>> + signal_is_valid = 1;
>> + break;
>> + }
>> + }
>> +
>> + if (signal_is_valid == 0) {
>> + return _libssh2_error(session, LIBSSH2_ERROR_INVALID_SIGNAL_NAME,
>> + "The signal name '%s' is invalid. Must be one of the following: "
>> + "ABRT, ALRM, FPE, HUP, ILL, INT, KILL, PIPE, QUIT, SEGV, TERM, "
>> + "USR1, USR2.",
>
> This list doesn't match the list in valid_signals[].
>
>> + signame
>> + );
>> + }
>> +
>> + if (channel->signal_state == libssh2_NB_state_idle) {
>> + /*
>> + * packet_type(1) + channel_id(4) + "signal"(6) + want_reply(1) +
>> + * signame(signame_len)
>> + */
>> + channel->signal_packet_len = 1 + 4 + 6 + 1 + signame_len;
>> +
>> + /* Zero the whole thing out */
>> + memset(&channel->signal_packet_requirev_state, 0,
>> + sizeof(channel->signal_packet_requirev_state));
>> +
>> + _libssh2_debug(session, LIBSSH2_TRACE_CONN,
>> + "Sending signal '%s' to channel %lu/%lu",
>> + signame,
>> + channel->local.id,
>> + channel->remote.id);
>> +
>> + unsigned char *s = channel->signal_packet =
>> + LIBSSH2_ALLOC(session, channel->signal_packet_len);
>> + if (!channel->signal_packet) {
>> + return _libssh2_error(session, LIBSSH2_ERROR_ALLOC,
>> + "Unable to allocate memeory "
>
> Typo: memeory
>
>>>> Dan
> _______________________________________________
> libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
>
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel