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
Received on 2011-10-17