Re: [hackers] [dwm] [PATCH] add a comment to clarify a potential overflow of ltsymbol
On Wed, Feb 15, 2023 at 05:28:43PM +, Tom Schwindl wrote: > Hi Hiltjo, > > > Hi, > > > > I think it is too verbose and don't think the comment is neccesary. > > > > I can understand the verbosity part, but that's something I can change. > My point here was more that bug reports get posted to the ML and I even > get them in private or from people I know in person. So I think a comment, > as you, yourself suggested, might be helpful. > A simple "Hey, if you're exceeding 16 bytes we cannot guarantee for anything" > would be enough (not in that tone, of course) for me. > > -- > Best Regards, > Tom Schwindl > > > -- > > Kind regards, > > Hiltjo > > > NO -- Kind regards, Hiltjo
Re: [hackers] [dwm] [PATCH] add a comment to clarify a potential overflow of ltsymbol
Hi Hiltjo, > Hi, > > I think it is too verbose and don't think the comment is neccesary. > I can understand the verbosity part, but that's something I can change. My point here was more that bug reports get posted to the ML and I even get them in private or from people I know in person. So I think a comment, as you, yourself suggested, might be helpful. A simple "Hey, if you're exceeding 16 bytes we cannot guarantee for anything" would be enough (not in that tone, of course) for me. -- Best Regards, Tom Schwindl > -- > Kind regards, > Hiltjo
Re: [hackers] [dwm] [PATCH] add a comment to clarify a potential overflow of ltsymbol
On Wed, Feb 15, 2023 at 11:29:26AM +, Tom Schwindl wrote:
> In case the strncpy() call is advised to copy >=16 characters, ltsymbol
> overflows.
> As dwm does not expect to have a ltsymbol bigger than 15 characters, there
> will be
> no length check[0]. Our target audience are programmers, they should be able
> to figure out how to extend the length by themselves. The reason to add this
> comment
> is that some inexperienced users are easily confused by this and the topic
> comes up
> from time to time[1].
>
> [0] https://lists.suckless.org/hackers/2208/18484.html
> [1] https://lists.suckless.org/dev/2210/35000.html
>
> ---
> I know that this patch looks like bikeshedding, but the comment is convenient.
> I also got messaged in private a few(!) times about that exact issue, but
> I'm not sure if I'm allowed to quote these mails here. Anyways, the point is
> that this issue arises from time to time and a comment would make our/their
> lifes easier.
>
> ---
> dwm.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/dwm.c b/dwm.c
> index c2bd8710544e..e400aa028587 100644
> --- a/dwm.c
> +++ b/dwm.c
> @@ -396,6 +396,9 @@ arrange(Monitor *m)
> void
> arrangemon(Monitor *m)
> {
> + /* dwm supports a ltsymbol size up to 15 chars (plus terminating
> NUL-byte).
> + * Anything greater than that will cause issues. If the user wants a
> name
> + * containing more chars, they need to modify the code to fit their
> needs */
> strncpy(m->ltsymbol, m->lt[m->sellt]->symbol, sizeof m->ltsymbol);
> if (m->lt[m->sellt]->arrange)
> m->lt[m->sellt]->arrange(m);
> --
> 2.39.1
>
>
Hi,
I think it is too verbose and don't think the comment is neccesary.
--
Kind regards,
Hiltjo
[hackers] [dwm] [PATCH] add a comment to clarify a potential overflow of ltsymbol
In case the strncpy() call is advised to copy >=16 characters, ltsymbol
overflows.
As dwm does not expect to have a ltsymbol bigger than 15 characters, there will
be
no length check[0]. Our target audience are programmers, they should be able
to figure out how to extend the length by themselves. The reason to add this
comment
is that some inexperienced users are easily confused by this and the topic
comes up
from time to time[1].
[0] https://lists.suckless.org/hackers/2208/18484.html
[1] https://lists.suckless.org/dev/2210/35000.html
---
I know that this patch looks like bikeshedding, but the comment is convenient.
I also got messaged in private a few(!) times about that exact issue, but
I'm not sure if I'm allowed to quote these mails here. Anyways, the point is
that this issue arises from time to time and a comment would make our/their
lifes easier.
---
dwm.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/dwm.c b/dwm.c
index c2bd8710544e..e400aa028587 100644
--- a/dwm.c
+++ b/dwm.c
@@ -396,6 +396,9 @@ arrange(Monitor *m)
void
arrangemon(Monitor *m)
{
+ /* dwm supports a ltsymbol size up to 15 chars (plus terminating
NUL-byte).
+* Anything greater than that will cause issues. If the user wants a
name
+* containing more chars, they need to modify the code to fit their
needs */
strncpy(m->ltsymbol, m->lt[m->sellt]->symbol, sizeof m->ltsymbol);
if (m->lt[m->sellt]->arrange)
m->lt[m->sellt]->arrange(m);
--
2.39.1
