The code looks OK to me, but it might be easier to have an extra local
variable (e.g. char *name) that is set to either sep->s_name or
tmpbuf; that would reduce the code duplication.  Maybe something like
the following (untested)?

diff --git a/ares_getnameinfo.c b/ares_getnameinfo.c
index 5b9f6386b29e..b0bc6da868cd 100644
--- a/ares_getnameinfo.c
+++ b/ares_getnameinfo.c
@@ -281,6 +281,8 @@ static char *lookup_service(unsigned short port, int flags,
   struct servent se;
 #endif
   char tmpbuf[4096];
+  char *name;
+  size_t name_len;

   if (port)
     {
@@ -323,14 +325,20 @@ static char *lookup_service(unsigned short port,
int flags,
 #endif
         }
       if (sep && sep->s_name)
-        /* get service name */
-        strcpy(tmpbuf, sep->s_name);
+        {
+          /* get service name */
+          name = sep->s_name;
+        }
       else
-        /* get port as a string */
-        sprintf(tmpbuf, "%u", (unsigned int)ntohs(port));
-      if (strlen(tmpbuf) < buflen)
+        {
+          /* get port as a string */
+          sprintf(tmpbuf, "%u", (unsigned int)ntohs(port));
+          name = tmpbuf;
+        }
+      name_len = strlen(name);
+      if (name_len < buflen)
         /* return it if buffer big enough */
-        strcpy(buf, tmpbuf);
+        memcpy(buf, name, name_len + 1);
       else
         /* avoid reusing previous one */
         buf[0] = '\0';

On Fri, Sep 19, 2014 at 7:51 PM, Gregor Jasny <gja...@googlemail.com> wrote:
> Fix Coverity error CID 56886.
>
> Signed-off-by: Gregor Jasny <gja...@googlemail.com>
> ---
>  ares_getnameinfo.c | 30 ++++++++++++++++++++----------
>  1 file changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/ares_getnameinfo.c b/ares_getnameinfo.c
> index 5b9f638..39337e7 100644
> --- a/ares_getnameinfo.c
> +++ b/ares_getnameinfo.c
> @@ -323,17 +323,27 @@ static char *lookup_service(unsigned short port, int 
> flags,
>  #endif
>          }
>        if (sep && sep->s_name)
> -        /* get service name */
> -        strcpy(tmpbuf, sep->s_name);
> -      else
> -        /* get port as a string */
> -        sprintf(tmpbuf, "%u", (unsigned int)ntohs(port));
> -      if (strlen(tmpbuf) < buflen)
> -        /* return it if buffer big enough */
> -        strcpy(buf, tmpbuf);
> +        {
> +          /* get service name */
> +          size_t name_len = strlen(sep->s_name);
> +          if (name_len < buflen)
> +            /* return it if buffer big enough */
> +            memcpy(buf, sep->s_name, name_len + 1);
> +          else
> +            /* avoid reusing previous one */
> +            buf[0] = '\0';
> +        }
>        else
> -        /* avoid reusing previous one */
> -        buf[0] = '\0';
> +        {
> +          /* get port as a string */
> +          sprintf(tmpbuf, "%u", (unsigned int)ntohs(port));
> +          if (strlen(tmpbuf) < buflen)
> +            /* return it if buffer big enough */
> +            strcpy(buf, tmpbuf);
> +          else
> +            /* avoid reusing previous one */
> +            buf[0] = '\0';
> +        }
>        return buf;
>      }
>    buf[0] = '\0';
> --
> 1.8.5.2 (Apple Git-48)
>

Reply via email to