Re: [hackers] [dwm][PATCH] ensure layout symbol is properly nul-terminated

From: Hiltjo Posthuma <hiltjo_AT_codemadness.org>
Date: Sat, 13 Aug 2022 09:55:00 +0200

On Sat, Aug 13, 2022 at 09:00:06AM +0600, NRK wrote:
> the dwm source code somewhat silently assumes that the symbol will fit
> into 16 bytes. but this may not be the case since users can define
> longer symbols in `config.h` (and there's no comment or note stating
> anything about the size requirement).
>
> this patch syncs the sizeof Layout->symbol and Monitor->ltsymbol to be
> the same which should emit a warning if the symbol in config.h is longer
> than 17 bytes.
>
> however, there's still an edge case where the symbol is exactly 17 bytes
> (including the nul character), in which case there won't be any
> warnings. so use stpncpy and manually ensure nul termination.
> ---
> dwm.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/dwm.c b/dwm.c
> index 967c9e8..86c4985 100644
> --- a/dwm.c
> +++ b/dwm.c
> _AT_@ -107,12 +107,12 @@ typedef struct {
> } Key;
>
> typedef struct {
> - const char *symbol;
> + const char symbol[16];
> void (*arrange)(Monitor *);
> } Layout;
>
> struct Monitor {
> - char ltsymbol[16];
> + char ltsymbol[sizeof ((Layout *)0)->symbol];
> float mfact;
> int nmaster;
> int num;
> _AT_@ -397,7 +397,7 @@ arrange(Monitor *m)
> void
> arrangemon(Monitor *m)
> {
> - strncpy(m->ltsymbol, m->lt[m->sellt]->symbol, sizeof m->ltsymbol);
> + *stpncpy(m->ltsymbol, m->lt[m->sellt]->symbol, sizeof m->ltsymbol - 1) = '\0';
> if (m->lt[m->sellt]->arrange)
> m->lt[m->sellt]->arrange(m);
> }
> _AT_@ -644,7 +644,7 @@ createmon(void)
> m->topbar = topbar;
> m->lt[0] = &layouts[0];
> m->lt[1] = &layouts[1 % LENGTH(layouts)];
> - strncpy(m->ltsymbol, layouts[0].symbol, sizeof m->ltsymbol);
> + *stpncpy(m->ltsymbol, layouts[0].symbol, sizeof m->ltsymbol - 1) = '\0';
> return m;
> }
>
> _AT_@ -1507,7 +1507,7 @@ setlayout(const Arg *arg)
> selmon->sellt ^= 1;
> if (arg && arg->v)
> selmon->lt[selmon->sellt] = (Layout *)arg->v;
> - strncpy(selmon->ltsymbol, selmon->lt[selmon->sellt]->symbol, sizeof selmon->ltsymbol);
> + *stpncpy(selmon->ltsymbol, selmon->lt[selmon->sellt]->symbol, sizeof selmon->ltsymbol - 1) = '\0';
> if (selmon->sel)
> arrange(selmon);
> else
> --
> 2.35.1
>
>

I think its simpler to just add the comment if needed. The patch looks overly complicated.

Because the configuration is part of the source-code I don't think there need
to be extensive checks necesarily. Users are treated as programmers.

(I remember a long time ago in AwesomeWM (using Lua) setting some high window
border size and it causing issues. But thats OK imho.)

-- 
Kind regards,
Hiltjo
Received on Sat Aug 13 2022 - 09:55:00 CEST

This archive was generated by hypermail 2.3.0 : Sat Aug 13 2022 - 10:00:38 CEST