There are additional warnings that indicate free is being called on pointers
that have not been properly allocated that I had not mentioned as of yet,
because I had yet to track them down... They are also potential security
concerns:
In function ‘afree’,
inlined from ‘afree’ at alloc.c:91:1,
inlined from ‘shf_sclose’ at shf.c:266:3,
inlined from ‘shf_sclose’ at shf.c:255:1,
inlined from ‘snptreef’ at tree.c:390:10:
alloc.c:100:17: warning: ‘free’ called on unallocated object ‘shf’
[-Wfree-nonheap-object]
100 | free(lp);
| ^
tree.c: In function ‘snptreef’:
tree.c:382:14: note: declared here
382 | struct shf shf;
| ^
In function ‘afree’,
inlined from ‘afree’ at alloc.c:91:1,
inlined from ‘shf_sclose’ at shf.c:266:3,
inlined from ‘shf_sclose’ at shf.c:255:1,
inlined from ‘wdstrip’ at tree.c:595:11,
inlined from ‘function_body’ at syn.c:537:10,
inlined from ‘get_command’ at syn.c:265:9:
alloc.c:100:17: warning: ‘free’ called on unallocated object ‘shf’
[-Wfree-nonheap-object]
100 | free(lp);
| ^
tree.c: In function ‘get_command’:
tree.c:582:20: note: declared here
582 | struct shf shf;
| ^
I finally tracked these down to functions where shf was being allocated on the
stack, then a pointer to that shf was being passed to shf_sopen()... the end
result is... "probably not what we want." When the pointer to the shf gets
passed to shf_sclose() at the end of these buggy methods, shf_sclose() calls
free() on the shf... and that's not good.
The shf_sopen() function is returning an allocated buffer... We should revise
the code to just use that allocation.
I have revised my patch and attached it with the all reported issues solved.
This code now compiles without warnings on CachyOS. I have the result installed
on my machine and am doing OK.
--- a/tree.c 2013-05-13 02:16:42.000000000 +0800
+++ b/tree.c 2025-12-02 23:48:17.139449919 +0800
@@ -2,6 +2,7 @@
* command tree climbing
*/
+#include <alloca.h>
#include "sh.h"
#define INDENT 4
@@ -379,26 +380,26 @@
snptreef(char *s, int n, const char *fmt, ...)
{
va_list va;
- struct shf shf;
+ struct shf *shf;
- shf_sopen(s, n, SHF_WR | (s ? 0 : SHF_DYNAMIC), &shf);
+ shf = shf_sopen(s, n, SHF_WR | (s ? 0 : SHF_DYNAMIC), NULL);
SH_VA_START(va, fmt);
- vfptreef(&shf, 0, fmt, va);
+ vfptreef(shf, 0, fmt, va);
va_end(va);
- return shf_sclose(&shf); /* null terminates */
+ return shf_sclose(shf); /* null terminates */
}
static void vfptreef(struct shf *shf, int indent, const char *fmt, va_list va)
{
int c;
+ char *buf = NULL; /* for integer -> string conversions */
+ const size_t sz = 24; /* safe for integer conversions */
while ((c = *fmt++))
if (c == '%') {
- long n;
- char *p, *q;
- int neg;
+ char *p;
switch ((c = *fmt++)) {
case 'c':
@@ -413,20 +414,21 @@
p = va_arg(va, char *);
tputS(p, shf);
break;
- case 'd': case 'u': /* decimal */
- n = (c == 'd') ? va_arg(va, int)
- : va_arg(va, unsigned int);
- neg = c=='d' && n<0;
- p = q = malloc(20);
- snprintf(p, 19, "%ld", (neg) ? -n : n);
- p[20] = '\0';
-
- if (neg)
- *--p = '-';
+ case 'd':
+ if (buf == NULL)
+ buf = alloca(sz);
+ p = buf;
+ snprintf(p, sz, "%d", va_arg(va, int));
+ while (*p)
+ tputc(*p++, shf);
+ break;
+ case 'u':
+ if (buf == NULL)
+ buf = alloca(sz);
+ p = buf;
+ snprintf(p, sz, "%u", va_arg(va, unsigned int));
while (*p)
tputc(*p++, shf);
-
- free(q);
break;
case 'T': /* format tree */
ptree(va_arg(va, struct op *), indent, shf);
@@ -579,10 +581,10 @@
wdstrip(wp)
const char *wp;
{
- struct shf shf;
+ struct shf *shf;
int c;
- shf_sopen((char *) 0, 32, SHF_WR | SHF_DYNAMIC, &shf);
+ shf = shf_sopen((char *) 0, 32, SHF_WR | SHF_DYNAMIC, NULL);
/* problems:
* `...` -> $(...)
@@ -592,52 +594,52 @@
while (1)
switch ((c = *wp++)) {
case EOS:
- return shf_sclose(&shf); /* null terminates */
+ return shf_sclose(shf); /* null terminates */
case CHAR:
case QCHAR:
- shf_putchar(*wp++, &shf);
+ shf_putchar(*wp++, shf);
break;
case COMSUB:
- shf_putchar('$', &shf);
- shf_putchar('(', &shf);
+ shf_putchar('$', shf);
+ shf_putchar('(', shf);
while (*wp != 0)
- shf_putchar(*wp++, &shf);
- shf_putchar(')', &shf);
+ shf_putchar(*wp++, shf);
+ shf_putchar(')', shf);
break;
case EXPRSUB:
- shf_putchar('$', &shf);
- shf_putchar('(', &shf);
- shf_putchar('(', &shf);
+ shf_putchar('$', shf);
+ shf_putchar('(', shf);
+ shf_putchar('(', shf);
while (*wp != 0)
- shf_putchar(*wp++, &shf);
- shf_putchar(')', &shf);
- shf_putchar(')', &shf);
+ shf_putchar(*wp++, shf);
+ shf_putchar(')', shf);
+ shf_putchar(')', shf);
break;
case OQUOTE:
break;
case CQUOTE:
break;
case OSUBST:
- shf_putchar('$', &shf);
+ shf_putchar('$', shf);
if (*wp++ == '{')
- shf_putchar('{', &shf);
+ shf_putchar('{', shf);
while ((c = *wp++) != 0)
- shf_putchar(c, &shf);
+ shf_putchar(c, shf);
break;
case CSUBST:
if (*wp++ == '}')
- shf_putchar('}', &shf);
+ shf_putchar('}', shf);
break;
#ifdef KSH
case OPAT:
- shf_putchar(*wp++, &shf);
- shf_putchar('(', &shf);
+ shf_putchar(*wp++, shf);
+ shf_putchar('(', shf);
break;
case SPAT:
- shf_putchar('|', &shf);
+ shf_putchar('|', shf);
break;
case CPAT:
- shf_putchar(')', &shf);
+ shf_putchar(')', shf);
break;
#endif /* KSH */
}
