Truncated thread names in pstree

[Rich Felker]

On Tue, Jun 18, 2013 at 05:40:14PM -0400, Mike Frysinger wrote:
> On Monday 03 June 2013 23:19:43 Rich Felker wrote:
> > On Fri, May 31, 2013 at 10:54:33PM -0400, Mike Frysinger wrote:
> > > On Friday 31 May 2013 20:34:16 Dag Wieers wrote:
> > > > --- busybox-1.21.0/procps/pstree.c.orig 2013-06-01 02:27:14.008530872
> > > > +0200 +++ busybox-1.21.0/procps/pstree.c      2013-06-01
> > > > 02:31:36.483537110 +0200 @@ -342,7 +342,7 @@
> > > > 
> > > >   static void handle_thread(const char *comm, pid_t pid, pid_t ppid,
> > > >   uid_t uid) {
> > > >          char threadname[COMM_LEN + 2];
> > > > -       sprintf(threadname, "{%.*s}", COMM_LEN - 2, comm);
> > > > +       sprintf(threadname, "{%.*s}", COMM_LEN - 1, comm);
> > > >          add_proc(threadname, pid, ppid, uid/*, 1*/);
> > > >   }
> > > 
> > > hmm, your change is correct all by itself (but really it should be using
> > > sizeof()), but i don't think the current code is correct.  it *should* be
> > > truncating 2 bytes from the comm name.
> > 
> > sizeof cannot be used directly in expressions for the * form of width
> > or precision, because it has the wrong type. The * requires int, and
> > sizeof gives size_t. If you use sizeof, you need to cast the result to
> > int.
> 
> in practice, it doesn't make a difference.  you're right that there should be 
> an (int) there to avoid a warning.
> -mike

It's not just a warning. Passing the wrong-size type to a variadic
function is UB, and depending on the calling convention in use, it may
shift the subsequent arguments to different registers or stack slots
and insert extra zeros in between if you pass size_t instead of int. I
suspect on most real-world archs it appears to work, but if so, that's
because you got lucky with how their ABIs allocate registers and stack
slots and/or their alignment requirements, not because the code is
just failing "to avoid a warning".

Rich