Gautam -

How does this even compile what you have sent?  You renamed
vtysh_client_config to vtysh_client_execute but didn't modify the calls of
vtysh_client_config to use it.  In addition you have introduced compiler
warnings that cause the build to fail with -werror turned on:

vtysh.c: In function ‘vtysh_exit_ripd_only’:
vtysh.c:236:5: error: passing argument 2 of ‘vtysh_client_execute’ discards
‘const’ qualifier from pointer target type [-Werror]
vtysh.c:92:1: note: expected ‘char *’ but argument is of type ‘const char *’
vtysh.c: In function ‘vtysh_execute_func’:
vtysh.c:357:10: error: passing argument 2 of ‘vtysh_client_execute’
discards ‘const’ qualifier from pointer target type [-Werror]
vtysh.c:92:1: note: expected ‘char *’ but argument is of type ‘const char *’
vtysh.c:398:10: error: passing argument 2 of ‘vtysh_client_execute’
discards ‘const’ qualifier from pointer target type [-Werror]
vtysh.c:92:1: note: expected ‘char *’ but argument is of type ‘const char *’
vtysh.c: In function ‘vtysh_write_terminal’:
vtysh.c:1720:5: error: implicit declaration of function
‘vtysh_client_config’ [-Werror=implicit-function-declaration]
vtysh.c: In function ‘vtysh_write_terminal_daemon’:
vtysh.c:1766:3: error: passing argument 2 of ‘vtysh_client_execute’
discards ‘const’ qualifier from pointer target type [-Werror]
vtysh.c:92:1: note: expected ‘char *’ but argument is of type ‘const char *’
cc1: all warnings being treated as errors
make: *** [vtysh.o] Error 1

You Added a new FILE *fp to vtysh_client_execute but all invocations of
vtysh_client_config set it to NULL?  Why?

Look inline for a few more comments.


On Tue, Sep 29, 2015 at 2:27 PM, Gautam Kumar <[email protected]> wrote:

> ---
>
> @@ -100,6 +100,7 @@ vtysh_client_config (struct vtysh_client *vclient,
> char *line)
>    int nbytes;
>    int i;
>    int readln;
> +  int numnulls = 0;
>
>    if (vclient->fd < 0)
>      return CMD_SUCCESS;
> @@ -145,112 +146,89 @@ vtysh_client_config (struct vtysh_client *vclient,
> char *line)
>           XFREE(MTYPE_TMP, buf);
>           return CMD_SUCCESS;
>         }
> -
> -      pbuf[nbytes] = '\0';
> -
> -      if (nbytes >= 4)
> -       {
> -         i = nbytes - 4;
> -         if (pbuf[i] == '\0' && pbuf[i + 1] == '\0' && pbuf[i + 2] ==
> '\0')
> -           {
> -             ret = pbuf[i + 3];
> -             break;
> -           }
> -       }
> -      pbuf += nbytes;
> -
> -      /* See if a line exists in buffer, if so parse and consume it, and
> -       * reset read position. */
> -      if ((eoln = strrchr(buf, '\n')) == NULL)
> -       continue;
> -
> -      if (eoln >= ((buf + bufsz) - 1))
> -       {
> -         fprintf (stderr, ERR_WHERE_STRING \
> -                  "warning - eoln beyond buffer end.\n");
> -       }
> -      vtysh_config_parse(buf);
> -
> -      eoln++;
> -      left = (size_t)(buf + bufsz - eoln);
> -      memmove(buf, eoln, left);
> -      buf[bufsz-1] = '\0';
> -      pbuf = buf + strlen(buf);
> +      else
>

The only exit case of the if () block above is return.  There is no need
for this else, please remove it.

donald


> +        {
> +           /* If we have already seen 3 nulls, then current byte is ret
> code */
> +           if ((numnulls == 3) && (nbytes == 1))
> +             {
> +                ret = pbuf[0];
> +                break;
> +             }
> +           pbuf[nbytes] = '\0';
> +           /* If the config needs to be written in file or stdout */
> +           if (fp)
> +           {
> +             fputs(pbuf, fp);
> +             fflush (fp);
> +           }
> +
> +           /* At max look last four bytes */
> +           if (nbytes >= 4)
> +           {
> +             i = nbytes - 4;
> +             numnulls = 0;
> +           }
> +           else
> +             i = 0;
> +
> +           /* Count the numnulls */
> +           while (i < nbytes && numnulls <3)
> +           {
> +             if (pbuf[i++] == '\0')
> +                numnulls++;
> +             else
> +                numnulls = 0;
> +           }
> +           /* We might have seen 3 consecutive nulls so store the ret
> code before updating pbuf*/
> +           ret = pbuf[nbytes-1];
> +           pbuf += nbytes;
> +
> +
> +           /* See if a line exists in buffer, if so parse and consume it,
> and
> +            * reset read position. If 3 nulls has been encountered
> consume the buffer before
> +            * next read.
> +            */
> +           if (((eoln = strrchr(buf, '\n')) == NULL) && (numnulls<3))
> +             continue;
> +
> +           if (eoln >= ((buf + bufsz) - 1))
> +           {
> +              fprintf (stderr, ERR_WHERE_STRING \
> +                   "warning - eoln beyond buffer end.\n");
> +           }
> +
> +           /* If the config needs parsing, consume it */
> +           if(!fp)
> +             vtysh_config_parse(buf);
> +
> +           eoln++;
> +           left = (size_t)(buf + bufsz - eoln);
> +           /*
> +            * This check is required since when a config line split
> between two consecutive reads,
> +            * then buf will have first half of config line and current
> read will bring rest of the
> +            * line. So in this case eoln will be 1 here, hence
> calculation of left will be wrong.
> +            * In this case we don't need to do memmove, because we have
> already seen 3 nulls.
> +            */
> +           if(left < bufsz)
> +             memmove(buf, eoln, left);
> +
> +           buf[bufsz-1] = '\0';
> +           pbuf = buf + strlen(buf);
> +           /* got 3 or more trailing NULs? */
> +           if ((numnulls >=3) && (i < nbytes))
> +           {
> +              break;
> +           }
> +        }
>      }
> -
>    /* Parse anything left in the buffer. */
> -
> -  vtysh_config_parse (buf);
> +  if(!fp)
> +    vtysh_config_parse (buf);
>
>    XFREE(MTYPE_TMP, buf);
>    return ret;
>  }
>
> -static int
> -vtysh_client_execute (struct vtysh_client *vclient, const char *line,
> FILE *fp)
> -{
> -  int ret;
> -  char buf[1001];
> -  int nbytes;
> -  int i;
> -  int numnulls = 0;
> -
> -  if (vclient->fd < 0)
> -    return CMD_SUCCESS;
> -
> -  ret = write (vclient->fd, line, strlen (line) + 1);
> -  if (ret <= 0)
> -    {
> -      vclient_close (vclient);
> -      return CMD_SUCCESS;
> -    }
> -
> -  while (1)
> -    {
> -      nbytes = read (vclient->fd, buf, sizeof(buf)-1);
> -
> -      if (nbytes <= 0 && errno != EINTR)
> -       {
> -         vclient_close (vclient);
> -         return CMD_SUCCESS;
> -       }
> -
> -      if (nbytes > 0)
> -       {
> -         if ((numnulls == 3) && (nbytes == 1))
> -           return buf[0];
> -
> -         buf[nbytes] = '\0';
> -         fputs (buf, fp);
> -         fflush (fp);
> -
> -         /* check for trailling \0\0\0<ret code>,
> -          * even if split across reads
> -          * (see lib/vty.c::vtysh_read)
> -          */
> -          if (nbytes >= 4)
> -            {
> -              i = nbytes-4;
> -              numnulls = 0;
> -            }
> -          else
> -            i = 0;
> -
> -          while (i < nbytes && numnulls < 3)
> -            {
> -              if (buf[i++] == '\0')
> -                numnulls++;
> -              else
> -                numnulls = 0;
> -            }
> -
> -          /* got 3 or more trailing NULs? */
> -          if ((numnulls >= 3) && (i < nbytes))
> -            return (buf[nbytes-1]);
> -       }
> -    }
> -}
> -
>  static void
>  vtysh_exit_ripd_only (void)
>  {
> @@ -1739,7 +1717,7 @@ DEFUN (vtysh_write_terminal,
>    vty_out (vty, "!%s", VTY_NEWLINE);
>
>    for (i = 0; i < array_size(vtysh_client); i++)
> -    vtysh_client_config (&vtysh_client[i], line);
> +    vtysh_client_config (&vtysh_client[i], line, NULL);
>
>    /* Integrate vtysh specific configuration. */
>    vtysh_config_write ();
> @@ -1812,7 +1790,7 @@ write_config_integrated(void)
>      }
>
>    for (i = 0; i < array_size(vtysh_client); i++)
> -    vtysh_client_config (&vtysh_client[i], line);
> +    vtysh_client_config (&vtysh_client[i], line, NULL);
>
>    vtysh_config_dump (fp);
>
> --
> 2.5.3
>
>
> _______________________________________________
> Quagga-dev mailing list
> [email protected]
> https://lists.quagga.net/mailman/listinfo/quagga-dev
>
_______________________________________________
Quagga-dev mailing list
[email protected]
https://lists.quagga.net/mailman/listinfo/quagga-dev

Reply via email to