[PATCH] sudo: print message if old protocol is used

[Pavel Březina]

I believe the patch explains itself.

[Simo Sorce]

On Thu, 2012-11-15 at 12:13 +0100, Pavel Březina wrote:
> @@ -185,9 +186,14 @@ static int sudosrv_cmd(enum sss_sudo_type type,
> struct cli_ctx *cli_ctx)
>      }
>  /* if protocol is invalid return */
> if (cli_ctx->cli_protocol_version->version !=
>
> SSS_SUDO_PROTOCOL_VERSION) {
>    DEBUG(SSSDBG_FATAL_FAILURE, ("Invalid protocol! [%d]\n",
>          cli_ctx->cli_protocol_version->version));
> if (protocol == 0) {
>    DEBUG(SSSDBG_FATAL_FAILURE, ("Protocol [%d] is not secure. "
>          "SSSD does not allow to use this protocol.\n",
>
> protocol));
>    ret = EFAULT;
>    goto done;
> } else if (protocol != SSS_SUDO_PROTOCOL_VERSION) {
>    DEBUG(SSSDBG_FATAL_FAILURE, ("Invalid protocol version [%d]!
>
> \n",
>                                 protocol));
>    ret = EFAULT;
>    goto done;

Nitpicking, but for this kind of situations I think a switch/case
statement would be much more clear (also avoids adding a variable as you
use it once):

switch(cli_ctx->cli_protocol_version->version) {
case 0:
    DEBUG(..);
    break;
case SSS_SUDO_PROTOCOL_VERSION:
    break;
default:
    DEBUG("invalid"...);
    break;
}

Simo.

[Pavel Březina]

On 11/15/2012 02:30 PM, Simo Sorce wrote:
> On Thu, 2012-11-15 at 12:13 +0100, Pavel Březina wrote:
> > @@ -185,9 +186,14 @@ static int sudosrv_cmd(enum sss_sudo_type type,
> > struct cli_ctx *cli_ctx)
> >       }
> >   /* if protocol is invalid return */
> > if (cli_ctx->cli_protocol_version->version !=
> >
> > SSS_SUDO_PROTOCOL_VERSION) {
> >    DEBUG(SSSDBG_FATAL_FAILURE, ("Invalid protocol! [%d]\n",
> >          cli_ctx->cli_protocol_version->version));
> > if (protocol == 0) {
> >    DEBUG(SSSDBG_FATAL_FAILURE, ("Protocol [%d] is not secure. "
> >          "SSSD does not allow to use this protocol.\n",
> >
> > protocol));
> >    ret = EFAULT;
> >    goto done;
> > } else if (protocol != SSS_SUDO_PROTOCOL_VERSION) {
> >    DEBUG(SSSDBG_FATAL_FAILURE, ("Invalid protocol version [%d]!
> >
> > \n",
> >                                 protocol));
> >     ret = EFAULT;
> >     goto done;
>
> Nitpicking, but for this kind of situations I think a switch/case
> statement would be much more clear (also avoids adding a variable as you
> use it once):
>
> switch(cli_ctx->cli_protocol_version->version) {
> case 0:
>      DEBUG(..);
>      break;
> case SSS_SUDO_PROTOCOL_VERSION:
>      break;
> default:
>      DEBUG("invalid"...);
>      break;
> }
>
> Simo.

OK. New patch is attached.

[Jakub Hrozek]

On Thu, Nov 15, 2012 at 05:54:26PM +0100, Pavel Březina wrote:
> > Nitpicking, but for this kind of situations I think a switch/case
> > statement would be much more clear (also avoids adding a variable as you
> > use it once):
> >
> > switch(cli_ctx->cli_protocol_version->version) {
> > case 0:
> >     DEBUG(..);
> >     break;
> > case SSS_SUDO_PROTOCOL_VERSION:
> >     break;
> > default:
> >     DEBUG("invalid"...);
> >     break;
> > }
> >
> > Simo.
>
> OK. New patch is attached.

Ack

[Jakub Hrozek]

On Thu, Nov 29, 2012 at 01:36:42PM +0100, Jakub Hrozek wrote:
> On Thu, Nov 15, 2012 at 05:54:26PM +0100, Pavel Březina wrote:
> > > Nitpicking, but for this kind of situations I think a switch/case
> > > statement would be much more clear (also avoids adding a variable as you
> > > use it once):
> > >
> > > switch(cli_ctx->cli_protocol_version->version) {
> > > case 0:
> > >     DEBUG(..);
> > >     break;
> > > case SSS_SUDO_PROTOCOL_VERSION:
> > >     break;
> > > default:
> > >     DEBUG("invalid"...);
> > >     break;
> > > }
> > >
> > > Simo.
> >
> > OK. New patch is attached.
>
> Ack

Pushed to master.