Re: [hackers] [sbase] [PATCH] *sum: Ignore -b and -t flags

2020-01-02 Thread Michael Forney
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.



Re: [hackers] [sbase] [PATCH] *sum: Ignore -b and -t flags

2020-01-02 Thread Michael Forney
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



[hackers] Unsubscribe

2020-01-02 Thread Abdullah

unsubscribe


signature.asc
Description: PGP signature


Re: [hackers] [sbase] [PATCH] *sum: Ignore -b and -t flags

2020-01-02 Thread Quentin Rameau
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]...



Re: [hackers] [sbase] [PATCH] *sum: Ignore -b and -t flags

2020-01-02 Thread Laslo Hunhold
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] [PATCH][sbase] sort: Don't do top-level sort when -c is used with -k

2020-01-02 Thread Richard Ipsum
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] ed: Use reallocarray || Michael Forney

2020-01-02 Thread Hiltjo Posthuma
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



[hackers] [dmenu] [PATCH] Fix GCC warning about using '*' in boolean context

2020-01-02 Thread Samee Zahur
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

2020-01-02 Thread k0ga
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,