[Bug c/25702] feature request: generate a warning for sizeof on a pointer

[Bug c/25702] feature request: generate a warning for sizeof on a pointer

[forgcc at calmarius dot net]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25702

David Csirmaz <forgcc at calmarius dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |forgcc at calmarius dot net

--- Comment #8 from David Csirmaz <forgcc at calmarius dot net> 2013-01-04 15:37:44 UTC ---
I encounter this mistake regularly too. 

But you have shown examples why shouldn't we need warning for that.

But I have a different idea. The sizeof operator is usually used to pass buffer
sizes to a function, along with pointer to the buffer itself.

So when you have a function call like:

dumpbuf(a, sizeof(a))

It's almost certainly wrong if `a` is a pointer, isn't it?

So basically the rule would be: if x and sizeof(x) is in the same expression
and x is a pointer, issue a warning.

[Bug c/25702] feature request: generate a warning for sizeof on a pointer

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25702

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-01-04 15:55:13 UTC ---
See -Wsizeof-pointer-memaccess warning added in GCC 4.8.  Currently it only
warns about a handful of builtin functions there is no attribute to annotate
user functions if they want similar behavior, but at least for various
memory/string functions, s*printf etc. you'll get warnings.

[Bug c/25702] feature request: generate a warning for sizeof on a pointer

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25702

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |mpolacek at gcc dot gnu.org
         Resolution|---                         |FIXED

--- Comment #10 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
I think -Wsizeof-pointer-memaccess should be enough for this PR be closed now.

[Bug c/25702] feature request: generate a warning for sizeof on a pointer

[meklund at cisco dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25702

--- Comment #11 from Mark Eklund <meklund at cisco dot com> ---
-Wsizeof-pointer-memaccess is definitely a good targeted fix and probably hits
a majority of what I've seen.  I'm good with this being resolved.

[Bug c/25702] feature request: generate a warning for sizeof on a pointer

[steven at gcc dot gnu dot org]

------- Comment #7 from steven at gcc dot gnu dot org  2009-08-08 22:35 -------
I believe this should be closed as WONTFIX.  Warnings exist to indicate things
in the program that are almost certainly wrong.  In this case, the only way to
really avoid false positives is to look at the context of the sizeof, which is
more something a static checker would do.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25702

[Bug c/25702] feature request: generate a warning for sizeof on a pointer

[pinskia at gcc dot gnu dot org]

------- Comment #6 from pinskia at gcc dot gnu dot org  2008-11-14 23:51 -------
Actually I use sizeof all the time on pointers so I don't think this is useful.
 In fact it falls down with meta programming.
That is:
#define bitcase(type, a) ({typeof (a) b = a; type c;  int
notthesamesize[sizeof(a) == sizeof(type)]; memcpy(&c, &a, sizeof(a)); c;})

And then type is some pointer type.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25702

[Bug c/25702] feature request: generate a warning for sizeof on a pointer

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marcus at jet dot franken
                   |                            |dot de
             Status|WAITING                     |NEW
     Ever Confirmed|0                           |1
           Keywords|                            |diagnostic
   Last reconfirmed|0000-00-00 00:00:00         |2008-01-26 12:10:39
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25702

[Bug c/25702] feature request: generate a warning for sizeof on a pointer

[meklund at cisco dot com]

------- Comment #5 from meklund at cisco dot com  2007-11-28 19:43 -------
Subject: Re:  feature request: generate a warning for sizeof on a pointer

Hi Manu,

This is in regards to your Comments for the gcc feature enhancement
request, http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25702.

On Wed, Nov 28, 2007 at 05:34:35PM -0000, manu at gcc dot gnu dot org wrote:
>
> ------- Comment #4 from manu at gcc dot gnu dot org  2007-11-28 17:34 -------
> So what the BSD people said about this? Did they agree with your assessment?
> How many of those 26 likely bugs were considered "real" bugs by them?

When I contacted Matt Dillon, he said all of them except two were
definitely bugs.  The other two were not in the Dragonfly code base,
so he didn't comment on those.  The 24 definite bugs have been
corrected in Dragonfly.

Paul Vixie corrected the bind bugs in the BIND8 sources and said
thought this check would be a great compiler feature.

I don't know if the bugs have been fixed in FreeBSD.  I contacted
security-officer@freebsd.org to make sure there were no security
problems cause by these bugs and got no response.  I didn't pursue the
matter further after that.

>
> It really seems too noisy and with no clear way to avoid or workaround an
> invalid warning.
>

This should be implemented in such a way that 'sizeof(ptr)' gets
warning, but 'sizeof(char *)' doesn't.

--mark


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25702

[Bug c/25702] feature request: generate a warning for sizeof on a pointer

[manu at gcc dot gnu dot org]

------- Comment #4 from manu at gcc dot gnu dot org  2007-11-28 17:34 -------
So what the BSD people said about this? Did they agree with your assessment?
How many of those 26 likely bugs were considered "real" bugs by them?

It really seems too noisy and with no clear way to avoid or workaround an
invalid warning.


-- 

manu at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |manu at gcc dot gnu dot org
             Status|UNCONFIRMED                 |WAITING


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25702

[Bug c/25702] feature request: generate a warning for sizeof on a pointer

[meklund at cisco dot com]

------- Comment #3 from meklund at cisco dot com  2006-01-17 15:36 -------
Subject: Re:  feature request: generate a warning for sizeof on a pointer

Using the FreeBSD latest CVS pull on 10-Jan-06 (5.4 based), a build world
was run with the below modifications to GCC.  The output was then evaluated
giving the following results.

   22991 normal sizeof operations.
   16564 sizeof(<typedef>) operations.
     803 fu **x; sizeof(*x) operations.
     396 fu *x; sizeof(x) operations.
   40754 total

The 396 "fu *x; sizeof(x)" type operations were then examined.  Note
that I am only concerning myself with the "fu *x; sizeof(x)"
operations.  I'm no longer suggesting issuing warning for "fu **x;
sizeof(*x)", which are commonly used in the sizeof(m)/sizeof(*m)
mentioned in my previous reply.  The "fu *x; sizeof(x)" operations are
broken down into the following categories:

1. (229) Calls within auto-generated files (mostly from cc_tools/gt*.[ch])
2. (74) Calls involving DEPRECATED_REGISTER_GDBARCH_SWAP macro
3. (59) Calls like read(n, &i, sizeof(i)) or bcopy(x, &i, sizeof(i))
4. (8) Misc seemingly correct calls.
5. (26) Likely bugs.

The likely bugs are listed after the below patch.

This means that 6.6% of what I want to display as optional warnings in well
established code is actually bugs.  If the auto-generated files are ignored,
15.6% were bugs.

--- c-common.c  2006/01/10 15:42:38     1.2
+++ c-common.c  2006/01/11 15:43:06
@@ -5905,4 +5905,17 @@
     error ("%s", string);
 }

+FILE *
+create_sizeof_stats_file(void)
+{
+  static FILE *sizeof_stats = NULL;
+  if (!sizeof_stats) {
+    char *sizeof_stats_fname = alloca(strlen(dump_base_name + 2 +
+                                             sizeof(".sizeof")));
+    sprintf(sizeof_stats_fname, "%s.sizeof", dump_base_name);
+    sizeof_stats = fopen(sizeof_stats_fname, "w");
+  }
+  return sizeof_stats;
+}
+
 #include "gt-c-common.h"
--- c-common.h  2006/01/10 15:42:38     1.2
+++ c-common.h  2006/01/11 15:43:07
@@ -1330,4 +1330,6 @@
 extern void pp_file_change (const struct line_map *);
 extern void pp_dir_change (cpp_reader *, const char *);

+extern FILE * create_sizeof_stats_file(void);
+
 #endif /* ! GCC_C_COMMON_H */
--- c-parse.in  2006/01/10 15:42:38     1.2
+++ c-parse.in  2006/01/11 16:43:30
@@ -518,9 +518,30 @@
                  if (TREE_CODE ($2) == COMPONENT_REF
                      && DECL_C_BIT_FIELD (TREE_OPERAND ($2, 1)))
                    error ("`sizeof' applied to a bit-field");
+                 static FILE *sizeof_stats = NULL;
+                  sizeof_stats = create_sizeof_stats_file();
+                  if (TREE_CODE (TREE_TYPE ($2)) == POINTER_TYPE) {
+                      if (TREE_CODE($2) == VAR_DECL) {
+                          fprintf(sizeof_stats, 
+                                  "%s: %d: MWE3:`sizeof' applied to a pointer"
+                                  " variable\n", input_filename, input_line);
+                      } else {
+                          fprintf(sizeof_stats, 
+                                  "%s: %d: MWE2:`sizeof' reference\n",
+                                  input_filename, input_line);
+                      }
+                  } else {
+                      fprintf(sizeof_stats, 
+                              "%s: %d: MWE0:`sizeof' reference\n",
+                              input_filename, input_line);
+                  }
                  $$ = c_sizeof (TREE_TYPE ($2)); }
        | sizeof '(' typename ')'  %prec HYPERUNARY
                { skip_evaluation--;
+                 static FILE *sizeof_stats = NULL;
+                  sizeof_stats = create_sizeof_stats_file();
+                  fprintf(sizeof_stats, "%s: %d: MWE1:`sizeof' reference\n",
+                          input_filename, input_line);
                  $$ = c_sizeof (groktypename ($3)); }
        | alignof unary_expr  %prec UNARY
                { skip_evaluation--;


---
/usr/src/gnu/usr.bin/binutils/readelf/../../../../contrib/binutils/binutils/readelf.c:
9569
Allocates too much memory.  (Also, never frees it.)
---
        int cnt;

        /* Find the section header so that we get the size.  */
        while (sect->sh_type != SHT_MIPS_OPTIONS)
        ++sect;

        eopt = get_data (NULL, file, options_offset, sect->sh_size,
                       _("options"));
        if (eopt)
        {
*         iopt = malloc ((sect->sh_size / sizeof (eopt)) * sizeof (*iopt));
          if (iopt == NULL)
            {
              error (_("Out of memory"));
              return 0;
            }

          offset = cnt = 0;
          option = iopt;

          while (offset < sect->sh_size)
---
/usr/src/gnu/usr.bin/gdb/libgdb/../../../../contrib/gdb/gdb/jv-valprint.c: 113
Assumes a 4 byte pointer.  See FIXME.
---
              char *buf;

              buf = alloca (TARGET_PTR_BIT / HOST_CHAR_BIT);
              fputs_filtered (", ", stream);
              wrap_here (n_spaces (2));

              if (i > 0)
                element = next_element;
              else
                {
*                 read_memory (address, buf, sizeof (buf));
                  address += TARGET_PTR_BIT / HOST_CHAR_BIT;
                  /* FIXME: cagney/2003-05-24: Bogus or what.  It
                       pulls a host sized pointer out of the target and
                       then extracts that as an address (while assuming
                       that the address is unsigned)!  */
                  element = extract_unsigned_integer (buf, sizeof (buf));
                }

              for (reps = 1; i + reps < length; reps++)
                {
---
/usr/src/gnu/usr.bin/gdb/libgdb/../../../../contrib/gdb/gdb/jv-valprint.c: 119
FIXME says it all.
---
              if (i > 0)
                element = next_element;
              else
                {
                  read_memory (address, buf, sizeof (buf));
                  address += TARGET_PTR_BIT / HOST_CHAR_BIT;
                  /* FIXME: cagney/2003-05-24: Bogus or what.  It
                       pulls a host sized pointer out of the target and
                       then extracts that as an address (while assuming
                       that the address is unsigned)!  */
*                 element = extract_unsigned_integer (buf, sizeof (buf));
                }

              for (reps = 1; i + reps < length; reps++)
                {
                  read_memory (address, buf, sizeof (buf));
                  address += TARGET_PTR_BIT / HOST_CHAR_BIT;
                  /* FIXME: cagney/2003-05-24: Bogus or what.  It
                       pulls a host sized pointer out of the target and
                       then extracts that as an address (while assuming
                       that the address is unsigned)!  */
---
/usr/src/gnu/usr.bin/gdb/libgdb/../../../../contrib/gdb/gdb/jv-valprint.c: 124
FIXME says it all.
---
                  address += TARGET_PTR_BIT / HOST_CHAR_BIT;
                  /* FIXME: cagney/2003-05-24: Bogus or what.  It
                       pulls a host sized pointer out of the target and
                       then extracts that as an address (while assuming
                       that the address is unsigned)!  */
                  element = extract_unsigned_integer (buf, sizeof (buf));
                }

              for (reps = 1; i + reps < length; reps++)
                {
*                 read_memory (address, buf, sizeof (buf));
                  address += TARGET_PTR_BIT / HOST_CHAR_BIT;
                  /* FIXME: cagney/2003-05-24: Bogus or what.  It
                       pulls a host sized pointer out of the target and
                       then extracts that as an address (while assuming
                       that the address is unsigned)!  */
                  next_element = extract_unsigned_integer (buf, sizeof (buf));
                  if (next_element != element)
                    break;
                }

---
/usr/src/gnu/usr.bin/gdb/libgdb/../../../../contrib/gdb/gdb/jv-valprint.c: 130
FIXME says it all.
---
                }

              for (reps = 1; i + reps < length; reps++)
                {
                  read_memory (address, buf, sizeof (buf));
                  address += TARGET_PTR_BIT / HOST_CHAR_BIT;
                  /* FIXME: cagney/2003-05-24: Bogus or what.  It
                       pulls a host sized pointer out of the target and
                       then extracts that as an address (while assuming
                       that the address is unsigned)!  */
*                 next_element = extract_unsigned_integer (buf, sizeof (buf));
                  if (next_element != element)
                    break;
                }

              if (reps == 1)
                fprintf_filtered (stream, "%d: ", i);
              else
                fprintf_filtered (stream, "%d..%d: ", i, i + reps - 1);

              if (element == 0)
---
/usr/src/lib/bind/bind/../../../contrib/bind9/lib/bind/irs/dns_ho.c: 263
Although the entire buffer pointed by 'q' eventually gets filled,
this should be changed to either "q->next = NULL;" or 
"memset(q, 0, sizeof(*q))"
---

        if (init(this) == -1)
                return (NULL);

        q = memget(sizeof(*q));
        if (q == NULL) {
                RES_SET_H_ERRNO(pvt->res, NETDB_INTERNAL);
                errno = ENOMEM;
                goto cleanup;
        }
*       memset(q, 0, sizeof(q));

        switch (af) {
        case AF_INET:
                size = INADDRSZ;
                q->qclass = C_IN;
                q->qtype = T_A;
                q->answer = q->qbuf.buf;
                q->anslen = sizeof(q->qbuf);
                q->action = RESTGT_DOALWAYS;
                break;
---
/usr/src/lib/bind/bind/../../../contrib/bind9/lib/bind/irs/dns_ho.c: 355
Although the entire buffer pointed by 'q' eventually gets filled,
this should be changed to either "q->next = NULL;" or 
"memset(q, 0, sizeof(*q))"
---
        if (init(this) == -1)
                return (NULL);

        q = memget(sizeof(*q));
        q2 = memget(sizeof(*q2));
        if (q == NULL || q2 == NULL) {
                RES_SET_H_ERRNO(pvt->res, NETDB_INTERNAL);
                errno = ENOMEM;
                goto cleanup;
        }
*       memset(q, 0, sizeof(q));
        memset(q2, 0, sizeof(q2));

        if (af == AF_INET6 && len == IN6ADDRSZ &&
            (!memcmp(uaddr, mapped, sizeof mapped) ||
             (!memcmp(uaddr, tunnelled, sizeof tunnelled) &&
              memcmp(&uaddr[sizeof tunnelled], v6local, sizeof(v6local))))) {
                /* Unmap. */
                addr = (const char *)addr + sizeof mapped;
                uaddr += sizeof mapped;
                af = AF_INET;
---
/usr/src/lib/bind/bind/../../../contrib/bind9/lib/bind/irs/dns_ho.c: 356
Although the entire buffer pointed by 'q2' eventually gets filled,
this should be changed to either "q2->next = NULL;" or 
"memset(q2, 0, sizeof(*q2))"
---
                return (NULL);

        q = memget(sizeof(*q));
        q2 = memget(sizeof(*q2));
        if (q == NULL || q2 == NULL) {
                RES_SET_H_ERRNO(pvt->res, NETDB_INTERNAL);
                errno = ENOMEM;
                goto cleanup;
        }
        memset(q, 0, sizeof(q));
*       memset(q2, 0, sizeof(q2));

        if (af == AF_INET6 && len == IN6ADDRSZ &&
            (!memcmp(uaddr, mapped, sizeof mapped) ||
             (!memcmp(uaddr, tunnelled, sizeof tunnelled) &&
              memcmp(&uaddr[sizeof tunnelled], v6local, sizeof(v6local))))) {
                /* Unmap. */
                addr = (const char *)addr + sizeof mapped;
                uaddr += sizeof mapped;
                af = AF_INET;
                len = INADDRSZ;
---
/usr/src/lib/bind/bind/../../../contrib/bind9/lib/bind/irs/dns_ho.c: 581
Although the entire buffer pointed by 'q' eventually gets filled,
this should be changed to either "q->next = NULL;" or 
"memset(q, 0, sizeof(*q))"
---
        memset(&sentinel, 0, sizeof(sentinel));
        cur = &sentinel;

        q = memget(sizeof(*q));
        q2 = memget(sizeof(*q2));
        if (q == NULL || q2 == NULL) {
                RES_SET_H_ERRNO(pvt->res, NETDB_INTERNAL);
                errno = ENOMEM;
                goto cleanup;
        }
*       memset(q, 0, sizeof(q2));
        memset(q2, 0, sizeof(q2));

        switch (pai->ai_family) {
        case AF_UNSPEC:
                /* prefer IPv6 */
                q->qclass = C_IN;
                q->qtype = T_AAAA;
                q->answer = q->qbuf.buf;
                q->anslen = sizeof(q->qbuf);
                q->next = q2;
---
/usr/src/lib/bind/bind/../../../contrib/bind9/lib/bind/irs/dns_ho.c: 582
Although the entire buffer pointed by 'q' eventually gets filled,
this should be changed to either "q->next = NULL;" or 
"memset(q, 0, sizeof(*q))"
---
        cur = &sentinel;

        q = memget(sizeof(*q));
        q2 = memget(sizeof(*q2));
        if (q == NULL || q2 == NULL) {
                RES_SET_H_ERRNO(pvt->res, NETDB_INTERNAL);
                errno = ENOMEM;
                goto cleanup;
        }
        memset(q, 0, sizeof(q2));
*       memset(q2, 0, sizeof(q2));

        switch (pai->ai_family) {
        case AF_UNSPEC:
                /* prefer IPv6 */
                q->qclass = C_IN;
                q->qtype = T_AAAA;
                q->answer = q->qbuf.buf;
                q->anslen = sizeof(q->qbuf);
                q->next = q2;
                q->action = RESTGT_DOALWAYS;
---
/usr/src/lib/libopie/../../contrib/opie/libopie/readpass.c: 255
This is a bug on machines the have 8 byte pointers.
---
      if (*(e++) == *c)
        goto error;

    e = erase;
    while(*e)
      if (*(e++) == *c) {
        if (c <= buf)
        goto beep;

        if (flags & 1)
*         write(1, bsseq, sizeof(bsseq) - 1);
        c--;
        goto loop;
      }

    e = kill;
    while(*e)
      if (*(e++) == *c) {
        if (c <= buf)
        goto beep;

---
/usr/src/lib/libopie/../../contrib/opie/libopie/readpass.c: 268
This is a bug on machines the have 8 byte pointers.
---
      }

    e = kill;
    while(*e)
      if (*(e++) == *c) {
        if (c <= buf)
        goto beep;

        if (flags & 1)
          while(c-- > buf)
*           write(1, bsseq, sizeof(bsseq) - 1);

        c = buf;
        goto loop;
      }

    if (c < end) {
      if (*c < 32)
        goto beep;
      if (flags & 1)
        write(1, c, 1);
---
/usr/src/lib/libsdp/service.c: 227
This is likely looking at the memory right after the pdu.  If that is true,
the wrong memory is being read.  sizeof(*pdu) should be used instead.
---
        pdu = (sdp_pdu_p) ss->rsp;
        pdu->tid = ntohs(pdu->tid);
        pdu->len = ntohs(pdu->len);

        if (pdu->pid != SDP_PDU_ERROR_RESPONSE || pdu->tid != ss->tid ||
            pdu->len < 2 || pdu->len != len - sizeof(*pdu)) {
                ss->error = EIO;
                return (-1);
        }

*       error  = (uint16_t) ss->rsp[sizeof(pdu)] << 8;
        error |= (uint16_t) ss->rsp[sizeof(pdu) + 1];

        if (error != 0) {
                ss->error = EIO;
                return (-1);
        }

        return (len);
  }

---
/usr/src/lib/libsdp/service.c: 228
This is likely looking at the memory right after the pdu.  If that is true,
the wrong memory is being read.  sizeof(*pdu) should be used instead.
---
        pdu->tid = ntohs(pdu->tid);
        pdu->len = ntohs(pdu->len);

        if (pdu->pid != SDP_PDU_ERROR_RESPONSE || pdu->tid != ss->tid ||
            pdu->len < 2 || pdu->len != len - sizeof(*pdu)) {
                ss->error = EIO;
                return (-1);
        }

        error  = (uint16_t) ss->rsp[sizeof(pdu)] << 8;
*       error |= (uint16_t) ss->rsp[sizeof(pdu) + 1];

        if (error != 0) {
                ss->error = EIO;
                return (-1);
        }

        return (len);
  }

---
/usr/src/lib/libtelnet/../../contrib/telnet/libtelnet/sra.c: 306
pk_encode accesses pass in 4 byte increments.  So the memset should
have zeroed out the entire password.  pk_encode could be reading random
values. 
---
                }
                break;

        case SRA_CONTINUE:
                if (passwd_sent) {
                        passwd_sent = 0;
                        printf("[ SRA login failed ]\r\n");
                        goto enc_user;
                }
                /* encode password */
*               memset(pass,0,sizeof(pass));
                telnet_gets("Password: ",pass,255,0);
                pk_encode(pass,xpass,&ck);
                /* send it off */
                if (auth_debug_mode)
                        printf("Sent KAB(P)\r\n");
                if (!Data(ap, SRA_PASS, (void *)xpass, strlen(xpass))) {
                        if (auth_debug_mode)
                                printf("Not enough room\r\n");
                        return;
                }
---
/usr/src/libexec/telnetd/../../contrib/telnet/telnetd/telnetd.c: 607
Although I don't know what size should be used for this strncpy, it is
doubtful this is what was wanted.
---
                     */
                    if (strncmp(first, terminaltype, sizeof(first)) == 0)
                        break;
                    /*
                     * Get the terminal name one more time, so that
                     * RFC1091 compliant telnets will cycle back to
                     * the start of the list.
                     */
                     _gettermname();
                    if (strncmp(first, terminaltype, sizeof(first)) != 0) {
*                       (void) strncpy(terminaltype, first,
sizeof(terminaltype)-1);
                        terminaltype[sizeof(terminaltype)-1] = '\0';
                    }
                    break;
                }
            }
        }
      }
      return(retval);
  }  /* end of getterminaltype */

---
/usr/src/libexec/telnetd/../../contrib/telnet/telnetd/telnetd.c: 608
Although I don't know what size should be used for this strncpy, it is
doubtful this is what was wanted.
---
                    if (strncmp(first, terminaltype, sizeof(first)) == 0)
                        break;
                    /*
                     * Get the terminal name one more time, so that
                     * RFC1091 compliant telnets will cycle back to
                     * the start of the list.
                     */
                     _gettermname();
                    if (strncmp(first, terminaltype, sizeof(first)) != 0) {
                        (void) strncpy(terminaltype, first,
sizeof(terminaltype)-1);
*                       terminaltype[sizeof(terminaltype)-1] = '\0';
                    }
                    break;
                }
            }
        }
      }
      return(retval);
  }  /* end of getterminaltype */

  static void
---
/usr/src/sbin/dhclient/common/../../../contrib/isc-dhcp/common/ctrace.c: 285
There is no documentation as to what the length referrs to.  I'm assuming
that it refers to the length of the seed characters.  On a machine that
has 8 byte pointers, the below would print an error.
---
                return;
        outseed = htonl (seed);
        trace_write_packet (ttype, sizeof outseed, (char *)&outseed, MDL);
        return;
  }

  void trace_seed_input (trace_type_t *ttype, unsigned length, char *buf)
  {
        u_int32_t *seed;

*       if (length != sizeof seed) {
                log_error ("trace_seed_input: wrong size (%d)", length);
        }
        seed = (u_int32_t *)buf;
        srandom (ntohl (*seed));
  }

  void trace_seed_stop (trace_type_t *ttype) { }
  #endif /* TRACING */
---
/usr/src/sbin/dhclient/common/../../../contrib/isc-dhcp/common/icmp.c: 294
This is attempting to compensate for the memory consumed by ia.
sizeof(*ia) should be used in the calculation.
---
  void trace_icmp_input_input (trace_type_t *ttype, unsigned length, char *buf)
  {
        struct iaddr *ia;
        unsigned len;
        u_int8_t *icbuf;
        ia = (struct iaddr *)buf;
        ia->len = ntohl(ia->len);
        icbuf = (u_int8_t *)(ia + 1);
        if (icmp_state -> icmp_handler)
                (*icmp_state -> icmp_handler) (*ia, icbuf,
*                                              (int)(length - sizeof ia));
  }

  void trace_icmp_input_stop (trace_type_t *ttype) { }

  void trace_icmp_output_input (trace_type_t *ttype, unsigned length, char
*buf)
  {
        struct icmp *icmp;
        struct iaddr ia;

        if (length != (sizeof (*icmp) + (sizeof ia))) {
---
/usr/src/sbin/kldconfig/kldconfig.c: 273
This mallocs a pointer and then uses that memory as a stucture.
Fun ensues.
---

  /* Break a string down into path components, store them into a queue */
  static void
  parsepath(struct pathhead *pathq, char *path, int uniq)
  {
        char *p;
        struct pathentry *pe;

        while ((p = strsep(&path, ";")) != NULL)
                if (!uniq) {
*                       if (((pe = malloc(sizeof(pe))) == NULL) ||
                            ((pe->path = strdup(p)) == NULL)) {
                                errno = ENOMEM;
                                err(1, "allocating path element");
                        }
                        TAILQ_INSERT_TAIL(pathq, pe, next);
                } else {
                        addpath(pathq, p, 1, 0);
                }
  }

---
/usr/src/usr.bin/netstat/mbuf.c: 119
This mallocs(sizeof *mbstat), reads sizeof mbstat, and then access the
structure members.  The read should for sizeof(*mbstat) or mlen.
---
        struct mbtypenames *mp;
        bool *seen = NULL;

        mlen = sizeof *mbstat;
        if ((mbstat = malloc(mlen)) == NULL) {
                warn("malloc: cannot allocate memory for mbstat");
                goto err;
        }

        if (mbaddr) {
*               if (kread(mbaddr, (char *)mbstat, sizeof mbstat))
                        goto err;
                if (kread(nmbcaddr, (char *)&nmbclusters, sizeof(int)))
                        goto err;
        } else {
                mlen = sizeof *mbstat;
                if (sysctlbyname("kern.ipc.mbstat", mbstat, &mlen, NULL, 0)
                    < 0) {
                        warn("sysctl: retrieving mbstat");
                        goto err;
                }
---
/usr/src/usr.bin/xlint/lint1/scan.l: 322
This should either memset the entire allocated buffer or do a
sb->sb_name = NULL.
---
  allocsb(void)
  {
        sbuf_t  *sb;

        if ((sb = sbfrlst) != NULL) {
                sbfrlst = sb->sb_nxt;
        } else {
                if ((sb = malloc(sizeof (sbuf_t))) == NULL)
                        nomem();
        }
*       (void)memset(sb, 0, sizeof (sb));
        return (sb);
  }

  /*
   * Put a sbuf structure to the free list
   */
  static void
  freesb(sbuf_t *sb)
  {

---
/usr/src/usr.sbin/ppp/ncpaddr.c: 872
The sizeof res should be the length of the res buffer.
---
    if (len >= NCP_ASCIIBUFFERSIZE - 1)
      return res;

    switch (range->ncprange_family) {
    case AF_INET:
      if (range->ncprange_ip4width == -1) {
        /* A non-contiguous mask */
        for (; len >= 3; res[len -= 2] = '\0')
          if (strcmp(res + len - 2, ".0"))
            break;
*       snprintf(res + len, sizeof res - len, "&0x%08lx",
                 (unsigned long)ntohl(range->ncprange_ip4mask.s_addr));
      } else if (range->ncprange_ip4width < 32)
        snprintf(res + len, sizeof res - len, "/%d", range->ncprange_ip4width);

      return res;

  #ifndef NOINET6
    case AF_INET6:
      if (range->ncprange_ip6width != 128)
        snprintf(res + len, sizeof res - len, "/%d", range->ncprange_ip6width);
---
/usr/src/usr.sbin/ppp/ncpaddr.c: 875
The sizeof res should be the length of the res buffer.
---
    switch (range->ncprange_family) {
    case AF_INET:
      if (range->ncprange_ip4width == -1) {
        /* A non-contiguous mask */
        for (; len >= 3; res[len -= 2] = '\0')
          if (strcmp(res + len - 2, ".0"))
            break;
        snprintf(res + len, sizeof res - len, "&0x%08lx",
                 (unsigned long)ntohl(range->ncprange_ip4mask.s_addr));
      } else if (range->ncprange_ip4width < 32)
*       snprintf(res + len, sizeof res - len, "/%d", range->ncprange_ip4width);

      return res;

  #ifndef NOINET6
    case AF_INET6:
      if (range->ncprange_ip6width != 128)
        snprintf(res + len, sizeof res - len, "/%d", range->ncprange_ip6width);

      return res;
  #endif
---
/usr/src/usr.sbin/ppp/ncpaddr.c: 882
The sizeof res should be the length of the res buffer.
---
        snprintf(res + len, sizeof res - len, "&0x%08lx",
                 (unsigned long)ntohl(range->ncprange_ip4mask.s_addr));
      } else if (range->ncprange_ip4width < 32)
        snprintf(res + len, sizeof res - len, "/%d", range->ncprange_ip4width);

      return res;

  #ifndef NOINET6
    case AF_INET6:
      if (range->ncprange_ip6width != 128)
*       snprintf(res + len, sizeof res - len, "/%d", range->ncprange_ip6width);

      return res;
  #endif
    }

    return "<AF_UNSPEC>";
  }

  #ifndef NOINET6
  int
---
/usr/src/usr.sbin/tcpdump/tcpdump/../../../contrib/tcpdump/print-ospf6.c: 351
This is likely checking if the memory goes beyond the end of available.
They probably want to do "lsap + 1" instead of "lsap + sizeof(lsapp)".  Also,
unless k becomes nonzero, I don't know how they get outof the loop.
---
                        printf(" %s", ipaddr_string(ap));
                        ++ap;
                }
                break;

        case LS_TYPE_INTER_AP | LS_SCOPE_AREA:
                TCHECK(lsap->lsa_un.un_inter_ap.inter_ap_metric);
                printf(" metric %u",
                       
EXTRACT_32BITS(&lsap->lsa_un.un_inter_ap.inter_ap_metric) & SLA_MASK_METRIC);
                lsapp = lsap->lsa_un.un_inter_ap.inter_ap_prefix;
*               while (lsapp + sizeof(lsapp) <= (struct lsa_prefix *)ls_end) {
                        k = ospf6_print_lsaprefix(lsapp);
                        if (k)
                                goto trunc;
                        lsapp = (struct lsa_prefix *)(((u_char *)lsapp) + k);
                }
                break;
        case LS_SCOPE_AS | LS_TYPE_ASE:
                TCHECK(lsap->lsa_un.un_asla.asla_metric);
                flags32 = EXTRACT_32BITS(&lsap->lsa_un.un_asla.asla_metric);
                ospf6_print_bits(ospf6_asla_flag_bits, flags32);


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25702

[Bug c/25702] feature request: generate a warning for sizeof on a pointer

[meklund at cisco dot com]

------- Comment #2 from meklund at cisco dot com  2006-01-06 22:24 -------
Subject: Re:  feature request: generate a warning for sizeof on a pointer

On Fri, Jan 06, 2006 at 10:12:55PM -0000, pinskia at gcc dot gnu dot org wrote:
> Actually people use sizeof(x) all the time to mean the correct thing, for an
> example: memcpy(&x, y, sizeof(x));

True, and that is why I'd like to make it an optional warning.  People
would be up in arms if it weren't optional.  But, for people that want
to avoid this easily missed problem, they could live with

  memcpy(&x, y, sizeof(xtype *))

I have seen one instance where people would consider it annoying:

  char *m[] = { "this", "is", "bothersome", "to", "some" };
  int m_items = sizeof(m) / sizeof(*m);

but once again, the avoidance of having unexpectedly short lengths
would override the annoyance for many.

How about I apply my patch and do a large build like BSD "make world"
and come back with a listing of how prevalent the above is?

--mark


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25702

[Bug c/25702] feature request: generate a warning for sizeof on a pointer

[pinskia at gcc dot gnu dot org]

------- Comment #1 from pinskia at gcc dot gnu dot org  2006-01-06 22:12 -------
Actually people use sizeof(x) all the time to mean the correct thing, for an
example: memcpy(&x, y, sizeof(x));


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25702