Re: [hackers] [sbase] ed: Use reallocarray || Michael Forney
Hi, On Tue, Dec 31, 2019 at 10:47:28PM +0100, g...@suckless.org wrote: > diff --git a/ed.c b/ed.c > index b844e86..e998e81 100644 > --- a/ed.c > +++ b/ed.c > @@ -204,7 +204,7 @@ makeline(char *s, int *off) > if (lastidx >= idxsize) { > lp = NULL; > if (idxsize <= SIZE_MAX - NUMLINES) > - lp = realloc(zero, (idxsize + NUMLINES) * sizeof(*lp)); > + lp = reallocarray(zero, idxsize + NUMLINES, sizeof(*lp)); reallocarray is not a posix function, so it shouldn't be used here. If you don't fell confortable using realloc due to the possibility of overflow then you should add an explicit check against it. Regards,
[hackers] [dmenu] [PATCH] Fix GCC warning about using '*' in boolean context
Tested on gcc --version: gcc (Ubuntu 9.2.1-9ubuntu2) 9.2.1 20191008 Was getting the following warning: dmenu.c: In function ‘setup’: dmenu.c:24:30: warning: ‘*’ in boolean context, suggest ‘&&’ instead [-Wint-in-bool-context] [...] dmenu.c:637:9: note: in expansion of macro ‘INTERSECT’ Warning no longer emitted after fix. --- dmenu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dmenu.c b/dmenu.c index 65f25ce..1e37a9a 100644 --- a/dmenu.c +++ b/dmenu.c @@ -634,7 +634,7 @@ setup(void) /* no focused window is on screen, so use pointer location instead */ if (mon < 0 && !area && XQueryPointer(dpy, root, &dw, &dw, &x, &y, &di, &di, &du)) for (i = 0; i < n; i++) - if (INTERSECT(x, y, 1, 1, info[i])) + if (INTERSECT(x, y, 1, 1, info[i]) != 0) break; x = info[i].x_org; -- 2.20.1
Re: [hackers] [sbase] ed: Use reallocarray || Michael Forney
On Thu, Jan 02, 2020 at 08:28:40AM +, k0ga wrote: > Hi, > > > On Tue, Dec 31, 2019 at 10:47:28PM +0100, g...@suckless.org wrote: > > diff --git a/ed.c b/ed.c > > index b844e86..e998e81 100644 > > --- a/ed.c > > +++ b/ed.c > > @@ -204,7 +204,7 @@ makeline(char *s, int *off) > > if (lastidx >= idxsize) { > > lp = NULL; > > if (idxsize <= SIZE_MAX - NUMLINES) > > - lp = realloc(zero, (idxsize + NUMLINES) * sizeof(*lp)); > > + lp = reallocarray(zero, idxsize + NUMLINES, sizeof(*lp)); > > reallocarray is not a posix function, so it shouldn't be used here. If you > don't fell confortable using realloc due to the possibility of overflow then > you should add an explicit check against it. > > Regards, > sbase has reallocarray in libutil: https://git.suckless.org/sbase/file/libutil/reallocarray.c.html -- Kind regards, Hiltjo
Re: [hackers] [PATCH][sbase] sort: Don't do top-level sort when -c is used with -k
On Wed, Jan 01, 2020 at 09:39:18PM -0800, Michael Forney wrote: > On 2020-01-01, Richard Ipsum wrote: [snip] > > I think the following diff should cover those cases as well: > > diff --git a/sort.c b/sort.c > index a51997f..fbb1abf 100644 > --- a/sort.c > +++ b/sort.c > @@ -385,7 +385,8 @@ main(int argc, char *argv[]) > /* -b shall only apply to custom key definitions */ > if (TAILQ_EMPTY(&kdhead) && global_flags) > addkeydef("1", global_flags & ~(MOD_STARTB | MOD_ENDB)); > - addkeydef("1", global_flags & MOD_R); > + if (TAILQ_EMPTY(&kdhead) || (!Cflag && !cflag)) > + addkeydef("1", global_flags & MOD_R); > > if (!argc) { > if (Cflag || cflag) { > > I'm still not convinced of the value of this tie-breaker keydef, so > another option might be to just only add it if no -k flag is > specified: > > diff --git a/sort.c b/sort.c > index a51997f..adf1d6d 100644 > --- a/sort.c > +++ b/sort.c > @@ -383,9 +383,8 @@ main(int argc, char *argv[]) > } ARGEND > > /* -b shall only apply to custom key definitions */ > - if (TAILQ_EMPTY(&kdhead) && global_flags) > + if (TAILQ_EMPTY(&kdhead)) > addkeydef("1", global_flags & ~(MOD_STARTB | MOD_ENDB)); > - addkeydef("1", global_flags & MOD_R); > > if (!argc) { > if (Cflag || cflag) { > > I think I will apply the first diff for now to fix the bug, and > perhaps propose the second as a patch to the list, since it involves > an unrelated behavior change (order of equal lines according to > options passed to sort). First diff looks good, as does the second, personally I don't feel too strongly either way about the presence of the tie-breaker, I think other sorts do something like it, so for this reason it may be worth keeping it. Thanks, Richard
Re: [hackers] [sbase] [PATCH] *sum: Ignore -b and -t flags
On Wed, 1 Jan 2020 22:04:47 -0800 Michael Forney wrote: Dear Michael, > These tools are not standardized, but these flags are supported in all > implementations I'm aware of (coreutils, busybox), and are > occasionally used in scripts. > > These flags are only meaningful on operating systems which > differentiate between text and binary files, so just ignore them. I would print something on stderr. POSIX is ignored often enough and a ton of scripts are using the cancerous GNU extensions and other extensions. If we just "ignore" them, there is no learning effect or push for change for script writers, so maybe we could add a warning while we ignore them, so when you run a script that makes use of these mostly useless flags (which we could also tell them), then this might a push in a good direction. What do you think? With best regards Laslo
Re: [hackers] [sbase] [PATCH] *sum: Ignore -b and -t flags
Hi Laslo, > > These tools are not standardized, but these flags are supported in all > > implementations I'm aware of (coreutils, busybox), and are > > occasionally used in scripts. > > > > These flags are only meaningful on operating systems which > > differentiate between text and binary files, so just ignore them. > > I would print something on stderr. POSIX is ignored often enough and a > ton of scripts are using the cancerous GNU extensions and other > extensions. If we just "ignore" them, there is no learning effect or > push for change for script writers, so maybe we could add a warning > while we ignore them, so when you run a script that makes use of these > mostly useless flags (which we could also tell them), then this might a > push in a good direction. What do you think? I agree in that silently ignoring commands from the user is bad, as it breaks expectations. Though as noted in this case, those are not standardized (maybe that wasn't a great idea to add them in sbase instead of ubase even if they can be implemented in a portable manner), so anything can happen there. I'm not sure we should start adding those kind of half-compability parsing with coreutils, where do we stop? Also for what it's worth: $ busybox md5sum -b md5sum.c md5sum: unrecognized option: b BusyBox v1.31.1 (2019-11-29 10:55:12 UTC) multi-call binary. Usage: md5sum [FILE]... $ busybox md5sum -t md5sum.c md5sum: unrecognized option: t BusyBox v1.31.1 (2019-11-29 10:55:12 UTC) multi-call binary. Usage: md5sum [FILE]...
[hackers] Unsubscribe
unsubscribe signature.asc Description: PGP signature
Re: [hackers] [sbase] [PATCH] *sum: Ignore -b and -t flags
On 2020-01-02, Quentin Rameau wrote: > I agree in that silently ignoring commands from the user is bad, as it > breaks expectations. Well, in this case ignoring them is a valid implementation, since on POSIX there is no distinction between opening a file as text or binary mode. So it will return the correct result with those flags. > Though as noted in this case, those are not standardized (maybe that > wasn't a great idea to add them in sbase instead of ubase even if they > can be implemented in a portable manner), so anything can happen there. There is http://austingroupbugs.net/view.php?id=1041 which encourages implementations to provide *some* method of checksum with high security, but unfortunately still has no specification of what that looks like. > I'm not sure we should start adding those kind of half-compability > parsing with coreutils, where do we stop? I disagree that it is "half-compatibility", but this is a good point. It's kind of a similar issue with tar(1), which was removed from POSIX, but still extremely widely used. I think the best we can do is implement the options that are commonly expected of the tools. In this case implementing -b and -t is as simple as adding a case to a switch, so I don't see much downside to doing so. > Also for what it's worth: > > $ busybox md5sum -b md5sum.c > md5sum: unrecognized option: b > BusyBox v1.31.1 (2019-11-29 10:55:12 UTC) multi-call binary. > > Usage: md5sum [FILE]... > > $ busybox md5sum -t md5sum.c > md5sum: unrecognized option: t > BusyBox v1.31.1 (2019-11-29 10:55:12 UTC) multi-call binary. > > Usage: md5sum [FILE]... Looks like with busybox the -b and -t options depend on ENABLE_FEATURE_MD5_SHA1_SUM_CHECK, which also enables the -c flag. $ busybox md5sum -b /dev/null d41d8cd98f00b204e9800998ecf8427e /dev/null
Re: [hackers] [sbase] [PATCH] *sum: Ignore -b and -t flags
On 2020-01-02, Laslo Hunhold wrote: > I would print something on stderr. POSIX is ignored often enough and a > ton of scripts are using the cancerous GNU extensions and other > extensions. If we just "ignore" them, there is no learning effect or > push for change for script writers, so maybe we could add a warning > while we ignore them, so when you run a script that makes use of these > mostly useless flags (which we could also tell them), then this might a > push in a good direction. What do you think? I'm well aware that POSIX is ignored often. I send patches to projects every time I run into a script using non-POSIX options that break compatibility with sbase. But in this case, POSIX is not really relevant since as I mentioned, these are not POSIX tools, and there aren't many different implementations (BSDs have their own md5(1), sha256(1), ...), so coreutils is essentially the standard. Who are we to decide that -b and -t are not be valid flags for sha*sum and should not be used? I ran into a script that used `sha256sum -b`, but how could I justify removing that option to upstream? The flags may be useless on operating systems that sbase supports, but the "rb" is a valid mode for C99 fopen ("r" opens as a text file), so there must be operating systems where it makes a difference. Removing -b may break the script on those operating systems.