Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/usr.bin/xlint lint: fix buffer truncation for type names



details:   https://anonhg.NetBSD.org/src/rev/9ba65aaf2c91
branches:  trunk
changeset: 948849:9ba65aaf2c91
user:      rillig <rillig%NetBSD.org@localhost>
date:      Sat Jan 02 03:49:25 2021 +0000

description:
lint: fix buffer truncation for type names

Previously, most type names had been cut off after 63 significant
characters.  In some cases, 127 characters survived, or 255.  And for
the debugging messages, sometimes even 1023.  This inconsistency was
useless.

It was wrong in the first place to make the caller of the function
tyname responsible for handling the buffer.  That's not something a
caller of such a simple function should do.  These callers have better
things to do.

The API of the new function type_name is as simple as possible.

In the implementation, the name of the type is generated anew each time.
I just didn't know whether the type details could change, once the type
is initialized, and I didn't want to find out.  To be on the safe side,
the resulting type name is cached, independently of the type it was
generated for.  Using a trivial, unbalanced binary tree should be good
enough for now.

All this work is necessary to support adding new debug logging, without
being distracted by irrelevant implementation details such as these
buffer sizes.  Adding new debug messages should be fun and easy; up to
now, it was overly bureaucratic.

diffstat:

 usr.bin/xlint/common/externs.h |    4 +-
 usr.bin/xlint/common/tyname.c  |  157 ++++++++++++++++++++++++++++++++++------
 usr.bin/xlint/lint1/decl.c     |   17 +--
 usr.bin/xlint/lint1/init.c     |   39 +++------
 usr.bin/xlint/lint1/tree.c     |  154 ++++++++++++++-------------------------
 usr.bin/xlint/lint2/chk.c      |   21 +---
 6 files changed, 216 insertions(+), 176 deletions(-)

diffs (truncated from 1018 to 300 lines):

diff -r aa64f178af28 -r 9ba65aaf2c91 usr.bin/xlint/common/externs.h
--- a/usr.bin/xlint/common/externs.h    Sat Jan 02 03:41:06 2021 +0000
+++ b/usr.bin/xlint/common/externs.h    Sat Jan 02 03:49:25 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: externs.h,v 1.9 2021/01/01 01:42:55 rillig Exp $       */
+/*     $NetBSD: externs.h,v 1.10 2021/01/02 03:49:25 rillig Exp $      */
 
 /*
  * Copyright (c) 1994, 1995 Jochen Pohl
@@ -44,7 +44,7 @@
 /*
  * tyname.c
  */
-extern const   char *tyname(char *, size_t, const type_t *);
+extern const char *type_name(const type_t *);
 extern int     sametype(const type_t *, const type_t *);
 extern const   char *tspec_name(tspec_t);
 
diff -r aa64f178af28 -r 9ba65aaf2c91 usr.bin/xlint/common/tyname.c
--- a/usr.bin/xlint/common/tyname.c     Sat Jan 02 03:41:06 2021 +0000
+++ b/usr.bin/xlint/common/tyname.c     Sat Jan 02 03:49:25 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: tyname.c,v 1.19 2021/01/02 01:36:28 rillig Exp $       */
+/*     $NetBSD: tyname.c,v 1.20 2021/01/02 03:49:25 rillig Exp $       */
 
 /*-
  * Copyright (c) 2005 The NetBSD Foundation, Inc.
@@ -35,7 +35,7 @@
 
 #include <sys/cdefs.h>
 #if defined(__RCSID) && !defined(lint)
-__RCSID("$NetBSD: tyname.c,v 1.19 2021/01/02 01:36:28 rillig Exp $");
+__RCSID("$NetBSD: tyname.c,v 1.20 2021/01/02 03:49:25 rillig Exp $");
 #endif
 
 #include <limits.h>
@@ -53,6 +53,103 @@
        } while (/*CONSTCOND*/0)
 #endif
 
+/* A tree of strings. */
+typedef struct name_tree_node {
+       char *ntn_name;
+       struct name_tree_node *ntn_less;
+       struct name_tree_node *ntn_greater;
+} name_tree_node;
+
+/* A growable string buffer. */
+typedef struct buffer {
+       size_t  len;
+       size_t  cap;
+       char *  data;
+} buffer;
+
+static name_tree_node *type_names;
+
+static name_tree_node *
+new_name_tree_node(const char *name)
+{
+       name_tree_node *n;
+
+       n = xmalloc(sizeof(*n));
+       n->ntn_name = xstrdup(name);
+       n->ntn_less = NULL;
+       n->ntn_greater = NULL;
+       return n;
+}
+
+/* Return the canonical instance of the string, with unlimited life time. */
+static const char *
+intern(const char *name)
+{
+       name_tree_node *n = type_names;
+       int cmp;
+
+       if (n == NULL) {
+               n = new_name_tree_node(name);
+               type_names = n;
+               return n->ntn_name;
+       }
+
+       while ((cmp = strcmp(name, n->ntn_name)) != 0) {
+               if (cmp < 0) {
+                       if (n->ntn_less == NULL) {
+                               n->ntn_less = new_name_tree_node(name);
+                               return n->ntn_less->ntn_name;
+                       }
+                       n = n->ntn_less;
+               } else {
+                       if (n->ntn_greater == NULL) {
+                               n->ntn_greater = new_name_tree_node(name);
+                               return n->ntn_greater->ntn_name;
+                       }
+                       n = n->ntn_greater;
+               }
+       }
+       return n->ntn_name;
+}
+
+static void
+buf_init(buffer *buf)
+{
+       buf->len = 0;
+       buf->cap = 128;
+       buf->data = xmalloc(buf->cap);
+       buf->data[0] = '\0';
+}
+
+static void
+buf_done(buffer *buf)
+{
+       free(buf->data);
+}
+
+static void
+buf_add(buffer *buf, const char *s)
+{
+       size_t len = strlen(s);
+
+       while (buf->len + len + 1 >= buf->cap) {
+               buf->data = xrealloc(buf->data, 2 * buf->cap);
+               buf->cap = 2 * buf->cap;
+       }
+
+       memcpy(buf->data + buf->len, s, len + 1);
+       buf->len += len;
+}
+
+static void
+buf_add_int(buffer *buf, int n)
+{
+       char num[1 + sizeof(n) * CHAR_BIT + 1];
+
+       snprintf(num, sizeof num, "%d", n);
+       buf_add(buf, num);
+}
+
 const char *
 tspec_name(tspec_t t)
 {
@@ -160,25 +257,28 @@
 }
 
 const char *
-tyname(char *buf, size_t bufsiz, const type_t *tp)
+type_name(const type_t *tp)
 {
-       tspec_t t;
-       const   char *s;
-       char lbuf[64];
-       char cv[20];
+       tspec_t t;
+       buffer buf;
+       const char *name;
 
        if (tp == NULL)
                return "(null)";
+
+       /*
+        * XXX: Why is this necessary, and in which cases does this apply?
+        * Shouldn't the type be an ENUM from the beginning?
+        */
        if ((t = tp->t_tspec) == INT && tp->t_isenum)
                t = ENUM;
 
-       s = tspec_name(t);
-
-       cv[0] = '\0';
+       buf_init(&buf);
        if (tp->t_const)
-               (void)strcat(cv, "const ");
+               buf_add(&buf, "const ");
        if (tp->t_volatile)
-               (void)strcat(cv, "volatile ");
+               buf_add(&buf, "volatile ");
+       buf_add(&buf, tspec_name(t));
 
        switch (t) {
        case BOOL:
@@ -208,37 +308,42 @@
        case LCOMPLEX:
        case SIGNED:
        case UNSIGN:
-               (void)snprintf(buf, bufsiz, "%s%s", cv, s);
                break;
        case PTR:
-               (void)snprintf(buf, bufsiz, "%s%s to %s", cv, s,
-                   tyname(lbuf, sizeof(lbuf), tp->t_subt));
+               buf_add(&buf, " to ");
+               buf_add(&buf, type_name(tp->t_subt));
                break;
        case ENUM:
-               (void)snprintf(buf, bufsiz, "%s%s %s", cv, s,
+               buf_add(&buf, " ");
 #ifdef t_enum
-                   tp->t_enum->etag->s_name
+               buf_add(&buf, tp->t_enum->etag->s_name);
 #else
-                   tp->t_isuniqpos ? "*anonymous*" : tp->t_tag->h_name
+               buf_add(&buf,
+                   tp->t_isuniqpos ? "*anonymous*" : tp->t_tag->h_name);
 #endif
-                   );
                break;
        case STRUCT:
        case UNION:
-               (void)snprintf(buf, bufsiz, "%s%s %s", cv, s,
+               buf_add(&buf, " ");
 #ifdef t_str
-                   tp->t_str->stag->s_name
+               buf_add(&buf, tp->t_str->stag->s_name);
 #else
-                   tp->t_isuniqpos ? "*anonymous*" : tp->t_tag->h_name
+               buf_add(&buf,
+                   tp->t_isuniqpos ? "*anonymous*" : tp->t_tag->h_name);
 #endif
-                   );
                break;
        case ARRAY:
-               (void)snprintf(buf, bufsiz, "%s%s of %s[%d]", cv, s,
-                   tyname(lbuf, sizeof(lbuf), tp->t_subt), tp->t_dim);
+               buf_add(&buf, " of ");
+               buf_add(&buf, type_name(tp->t_subt));
+               buf_add(&buf, "[");
+               buf_add_int(&buf, tp->t_dim);
+               buf_add(&buf, "]");
                break;
        default:
                LERROR("tyname(%d)", t);
        }
-       return buf;
+
+       name = intern(buf.data);
+       buf_done(&buf);
+       return name;
 }
diff -r aa64f178af28 -r 9ba65aaf2c91 usr.bin/xlint/lint1/decl.c
--- a/usr.bin/xlint/lint1/decl.c        Sat Jan 02 03:41:06 2021 +0000
+++ b/usr.bin/xlint/lint1/decl.c        Sat Jan 02 03:49:25 2021 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: decl.c,v 1.97 2021/01/01 14:11:20 rillig Exp $ */
+/* $NetBSD: decl.c,v 1.98 2021/01/02 03:49:25 rillig Exp $ */
 
 /*
  * Copyright (c) 1996 Christopher G. Demetriou.  All Rights Reserved.
@@ -38,7 +38,7 @@
 
 #include <sys/cdefs.h>
 #if defined(__RCSID) && !defined(lint)
-__RCSID("$NetBSD: decl.c,v 1.97 2021/01/01 14:11:20 rillig Exp $");
+__RCSID("$NetBSD: decl.c,v 1.98 2021/01/02 03:49:25 rillig Exp $");
 #endif
 
 #include <sys/param.h>
@@ -253,8 +253,7 @@
 {
        tspec_t t;
 #ifdef DEBUG
-       char buf[1024];
-       printf("%s: %s\n", __func__, tyname(buf, sizeof(buf), tp));
+       printf("%s: %s\n", __func__, type_name(tp));
 #endif
        if (tp->t_typedef) {
                /*
@@ -505,7 +504,6 @@
 {
        str_t *sp;
        sym_t *mem;
-       char buf[256];
 
        switch (tp->t_tspec) {
        case STRUCT:
@@ -527,7 +525,7 @@
                break;
        default:
                /* %s attribute ignored for %s */
-               warning(326, "packed", tyname(buf, sizeof(buf), tp));
+               warning(326, "packed", type_name(tp));
                break;
        }
 }
@@ -731,8 +729,7 @@
        scl = dcs->d_scl;
 
 #ifdef DEBUG
-       char buf[1024];
-       printf("%s: %s\n", __func__, tyname(buf, sizeof(buf), tp));
+       printf("%s: %s\n", __func__, type_name(tp));
 #endif
        if (t == NOTSPEC && s == NOTSPEC && l == NOTSPEC && c == NOTSPEC &&
            tp == NULL)
@@ -1109,10 +1106,8 @@
                    t == SHORT || t == USHORT || t == ENUM) {
                        if (bitfieldtype_ok == 0) {
                                if (sflag) {
-                                       char buf[64];
                                        /* bit-field type '%s' invalid ... */
-                                       warning(273,
-                                           tyname(buf, sizeof(buf), tp));
+                                       warning(273, type_name(tp));



Home | Main Index | Thread Index | Old Index