Re: [hackers] [sbase][PATCH] README: Add usage instruction for sbase-box. Fix typos.
On Fri, Sep 13, 2024 at 05:20:31AM +, Brandon Pribula wrote: > Hello, > > I thought it would be a good idea to add a usage instruction for sbase-box in > sbase's README similar to the one in ubase's README. > > Also, I found a couple of minor typos that I thought I would fix. There's no > command named 'sha238sum' but there is one named 'sha384sum' and in the > alphabetized list of commands 'paste' should come before 'pathchk'. > > For commit d458fa2 > > Thanks Applied, thanks (and sorry for the delay).
Re: [hackers] [sbase][PATCH] fix: update man pages to standard mdoc date format
Hi, On Sat, Sep 07, 2024 at 10:57:31PM +, Brandon Pribula wrote: > Hello, > > When viewing sbase's man pages the date displayed at the bottom is the > current date rather than the date entered in the .1 file. > > According to this: > > https://mandoc.bsd.lv/mdoc/details/date.html > > The traditional man date format used by sbase (.Dd year-month-day) is no > longer recommended and no longer supported by GNU troff or Heirloom Docs and > as a result the current date is displayed instead. > Although this format is still accepted by mandoc for backward compatibility. > > For portability it states the standard mdoc date format should be used > instead: .Dd month day, year > > I applied the following patch to sbase's last commit b30fb56 and it fixes the > issue. Applied, thanks Roberto Vargas
Re: [hackers] [st][PATCH] fix BadMatch error when embedding on some windows
Hi, On Wed, Aug 07, 2024 at 11:55:59PM -0300, Lucas de Sena wrote: > When embedded, st fails with BadMatch error if the embedder's window has > non-default colormap/depth/visual. This commit fixes that by creating > st's window inside root and then reparent it into embedder. > > The reference window for dc.gc is also changed to match root's visuals. > > A similar commit had been made for dmenu[1]. > See this issue[2] on github for context. With a fast review, this seems right to me. Regards,
Re: [hackers] [st][PATCH] remove secondary call to select
Hi, On Sat, Jun 29, 2024 at 12:21:51PM +0200, Hiltjo Posthuma wrote: > Hi Jeremy, > > Thanks for the updated patch and example. > > The example below doesn't seem to deadlock for me however. > > More eyes and review would be appreciated, If my memory does not betrayes me I think this was the problem that I tried to fix many years ago (pasting huge block of text). While I didn't try by myself now, I think he is right (or at least it was right some time ago). Regards,
Re: [hackers] [st][PATCH] remove secondary call to select
Hi, On Thu, Jun 27, 2024 at 06:51:28PM -0700, Jeremy Bobbin wrote: > because underlying programs can request info from the tty, there was a > potential recursion in ttyread: > 1. ttyread > 2. tty program wants window size > 3. ttywrite the window size > > this assumes the ttyfd is both ready for read & write. > > if, for example, the tty wants st to report the cursor position & the > write pipe is already cloggled, then this could cause a deadlock. > > this was initialy addressed with 2 selects: Yes, this is a shame, and I began a branch many years ago to remove it. We should have only one select because it is not so hard to get a deadlock. I cannot give an example now, but I found different ways to do it. Sadly, to fix this problem it is required deep changes in the code of st (of course, my old branch dead in some old computer that I don't remembre where it lives/deads now). Regards,
Re: [hackers] [st][PATCH] sgr-patch
Hi, On Mon, Apr 29, 2024 at 08:18:00PM +0100, Mikhail Kot wrote: > Sorry, didn't see your message. > SGR allows you to specify color delimiters in two way: colon > or semicolon. The semicolon version is now legacy one, and many > applications, e.g. aerc (its library vaxis-ui, to be precise) > use colons by default. I believe it's better to support both > versions While I am not against adding this patch, only notice that any application that uses directly sequences without consulting terminfo or termcap is broken. If vaxis-ui sends SGR with colons blindly then it should be fixed. Said that, as the change does not have a big impact, I would add it. Regards, > >
Re: [hackers] [sbase][PATCH] Add implementation of tac(1)
Hi, As this is a topic more about sbase/ubase organization more than about patches I am going to move the discussion to the dev mailing list. Please, answer there instead of here in hackers. Regards,
Re: [hackers] [sbase][PATCH] Add implementation of tac(1)
Hi, I was thinking about what to do with these patches adding new commands. They raised a concern about what should be the scope of sbase. The idea of sbase was to provide a minimal portable POSIX base, while having ubase for the POSIX commands that cannot be implemented in a portable way. Saying that, it is obvious that there are programs in sbase that are not part of POSIX: - md5sum - sha256sum - sha384sum - sha512sum - sponge - sync - tar - install and maybe some others. At this point is not clear to me what to do with tools like tac or shuf. There was a small discussion about this topic in the irc channel, and it was proposed to add a 3rd repository to contain all these tools that are not part of POSIX (a bit like the moretools package). From my point of view, the main drawback of it is that it requires a 3rd -box program (currently we have sbase-box and ubase-box). The current situation it not good, because the two -box are not sharing the library, and the disk space is duplicated (main reason of -box is to minimize disk space for restricted environments), and a 3rd -box would make the situation even worse. But in the other hand, I don't want to add more non POSIX tools in sbase (in fact, I personally would like to remove the current non POSIX tools). I would like to move the discussion here and see what alternatives we have and how to proceed in this case. Regards,
Re: [hackers] [ubase][PATCH 1/4] su: simplify logic
Hi, On Thu, Mar 07, 2024 at 02:52:49PM -0500, neeshy wrote: > On Thu Mar 7, 2024 at 1:19 PM EST, Roberto E. Vargas Caballero wrote: > > I think it makes it simpler while keeping the correct behaviour that I > > broke. > > Looks good to me! Pushed!
Re: [hackers] [ubase][PATCH 1/4] su: simplify logic
Hi, On Thu, Mar 07, 2024 at 02:18:28AM -0500, neeshy wrote: > It seems that the modifications you made break the use case where su is > called without a username. It would normally default to the root > user, but now it invokes usage() instead. My original patch worked as > intended. Could you rebase using the original patch instead? Thank you. > Sadly, I cannot rebase it as it is already in the central repository. What do you think about adding a new change like this: diff --git a/su.c b/su.c index eb8bea7..8eea82b 100644 --- a/su.c +++ b/su.c @@ -26,7 +26,7 @@ usage(void) int main(int argc, char *argv[]) { - char *usr = "root", *pass; + char *usr, *pass; char *shell, *envshell, *term; struct passwd *pw; char *newargv[3]; @@ -43,9 +43,14 @@ main(int argc, char *argv[]) usage(); } ARGEND; - if (argc != 1) + if (argc > 1) usage(); - usr = argv[0]; + usr = argc > 0 ? argv[0] : "root"; errno = 0; pw = getpwnam(usr); I think it makes it simpler while keeping the correct behaviour that I broke. Kind Regards, Roberto Vargas
Re: [hackers] [sbase][PATCH] tar: chktar: fix conditional typo
Hi, On Tue, Mar 05, 2024 at 09:20:57PM +0100, Elie Le Vaillant wrote: > --- > tar.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tar.c b/tar.c > index 0361b63..405b8d9 100644 > --- a/tar.c > +++ b/tar.c > @@ -423,7 +423,7 @@ chktar(struct header *h) > goto bad; > } > memcpy(tmp, h->chksum, sizeof(tmp)); > - for (i = 0; i < sizeof(tmp), tmp[i] == ' '; i++); > + for (i = 0; i < sizeof(tmp) && tmp[i] == ' '; i++); > for (; i < sizeof(tmp); i++) > if (tmp[i] == ' ') > tmp[i] = '\0'; Applied, thanks.
Re: [hackers] [ubase][PATCH 1/4] su: simplify logic
Hi, On Mon, Feb 12, 2024 at 04:25:49PM -0500, neeshy wrote: > Inline dologin, and simplify common code I have applied the 4 patches with a minor modification to the 1st one. Kind regards, Roberto Vargas.
Re: [hackers] [sbase][PATCH v3] tar: sanitize, chktar: leading spaces should be skipped over
Hi, On Sun, Feb 11, 2024 at 09:26:14AM +0100, Elie Le Vaillant wrote: > Some tar archives (eg. ftp://ftp.gnu.org/gnu/shtool/shtool-2.0.8.tar.gz) > use leading spaces instead of leading zeroes for numeric fields. > Although it is not allowed by the ustar specification, most tar > implementations recognize it as correct. But since 3ef6d4e4, we > replace all spaces by NULs here, not just trailing ones, which leads to > recognizing such archives as malformed. This fixes it: we now skip > over leading spaces, allowing strtol(3) to read those numeric fields. Applied, thanks.
Re: [hackers] [sbase][PATCH v2] tar: sanitize: leading zeros should be recognized
On Tue, Mar 05, 2024 at 01:16:36PM +0100, Roberto E. Vargas Caballero wrote: > Hi, > > On Sat, Feb 10, 2024 at 11:57:38PM +0100, Elie Le Vaillant wrote: > > @@ -399,10 +400,17 @@ sanitize(struct header *h) > > for (i = 0; i < LEN(fields); i++) > > - for (j = 0; j < fields[i].l; j++) > > + for (leading = 1, j = 0; j < fields[i].l; j++) > > if (fields[i].f[j] == ' ') > > - fields[i].f[j] = '\0'; > > + fields[i].f[j] = leading ? '0' : '\0'; > > + else > > + leading = 0; > > } > > What do you think if we move this loop into a function and we use > it in both cases? Ok, I saw that there is a new version of the patch. I am going to apply v3 without changes. sorry for the noise. Roberto Vargas
Re: [hackers] [sbase][PATCH v2] tar: sanitize: leading zeros should be recognized
Hi, On Sat, Feb 10, 2024 at 11:57:38PM +0100, Elie Le Vaillant wrote: > @@ -399,10 +400,17 @@ sanitize(struct header *h) > for (i = 0; i < LEN(fields); i++) > - for (j = 0; j < fields[i].l; j++) > + for (leading = 1, j = 0; j < fields[i].l; j++) > if (fields[i].f[j] == ' ') > - fields[i].f[j] = '\0'; > + fields[i].f[j] = leading ? '0' : '\0'; > + else > + leading = 0; > } What do you think if we move this loop into a function and we use it in both cases? Roberto Vargas
[hackers] Penfing patches for sbase and ubase
Hi, I know that there are some pending patches for sbase and ubase, but I am a bit busy these days and I will not be able to look a bit deeper on them until next week. Be patient until then :) Thank you
Re: [hackers] [st] Fix cursor move with wide glyphs || Quentin Rameau
Hi, On Sun, Feb 25, 2024 at 11:57:03AM +0100, g...@suckless.org wrote: > st would always move back 1 column, > even with wide glyhps (using more than a single column). > > The glyph rune is set on its first column, > and the other ones are to 0, > so loop until we detect the start of the previous glyph. Should this be done in every cursor addressing command? What happens if we position the cursor in the middle of a glyph? Kind regards,
Re: [hackers] [sbase][PATCH] expr: tonum: handle case where result was previously calculated
Hi, On Mon, Jan 22, 2024 at 02:18:10PM -0700, Randy Palamar wrote: > As pointed out in a mail to dev expr was segfaulting when multiple > math operations were specified on the command line: eg. 'expr 3 \* > 2 + 1'. This happens because the tonum(), introduced in e50d533, > assumed that v->str was always non null. parse() guarantees this > for user input but this is not the case when doop() is called with > the result of a previous calculation. However in that case we know > that v->num is already valid so we can simply return. > --- Applied, thanks.
Re: [hackers] [sbase][PATCH 2/2] expr: don't evaluate matched substr as a number
Hi, On Sun, Jan 07, 2024 at 11:02:18AM -0700, Randy Palamar wrote: > POSIX specifies that if the pattern contains a subexpression then > the first matched subexpression should be returned if it exists. > > This fixes things like the following: > > ./expr 3 : '\(.*\)' > Before: 3 > After: 3 Applied, thanks! Roberto Vargas
Re: [hackers] [sbase][PATCH 1/2] expr: treat expressions as strs until evaluation
Hi On Sun, Jan 07, 2024 at 11:02:17AM -0700, Randy Palamar wrote: > Comparison operations (>, <, =, etc.) and matching operations must > operate originally provided string not one that has gone back and > forth through string formatting. This caused operations such as > the following to give incorrect results: > > ./expr 3 : '.*' > Before: 1 > After: 5 > > This commit fixes that issue. Applied, thanks! Roberto Vargas.
[hackers] [PATCH 5/6] ed: Simplify sighup dealing
As we already have the dump() function we can move the modification check inside the new dump() function. --- ed.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ed.c b/ed.c index eaa4ca9..0705beb 100644 --- a/ed.c +++ b/ed.c @@ -710,6 +710,9 @@ dump(void) { char *home; + if (modflag) + return; + line1 = nextln(0); line2 = lastln; @@ -730,9 +733,8 @@ static void chksignals(void) { if (hup) { - if (modflag) - dump(); exstatus = 1; + dump(); quit(); } -- 2.37.3
[hackers] [PATCH 6/6] ed: Don't undo commands in sigint
If newcmd is 0 then error() undo all the modifications that happened since the last command, but this is not what POSIX mandates: SIGINT The ed utility shall interrupt its current activity, write the string "?\n" to standard output, and return to command mode (see the EXTENDED DESCRIPTION section). --- ed.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ed.c b/ed.c index 0705beb..4cba483 100644 --- a/ed.c +++ b/ed.c @@ -740,6 +740,7 @@ chksignals(void) if (intr) { intr = 0; + newcmd = 1; clearerr(stdin); error("Interrupt"); } -- 2.37.3
[hackers] [PATCH 2/6] ed: Remove nothing comments
Several bugs happened in the past due to this kind of comments and it is better to get rid of them. --- ed.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ed.c b/ed.c index b6f4f1c..b94afa5 100644 --- a/ed.c +++ b/ed.c @@ -199,7 +199,7 @@ makeline(char *s, int *off) len = 0; } else { while ((c = *s++) && c != '\n') - /* nothing */; + ; len = s - begin; if ((lp->seek = lseek(scratch, 0, SEEK_END)) < 0 || write(scratch, begin, len) < 0) { @@ -482,7 +482,7 @@ skipblank(void) char c; while ((c = input()) == ' ' || c == '\t') - /* nothing */; + ; back(c); } -- 2.37.3
[hackers] [PATCH 3/6] ed: Fix G and V commands
--- ed.c | 30 +++--- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/ed.c b/ed.c index b94afa5..35fddf1 100644 --- a/ed.c +++ b/ed.c @@ -1242,7 +1242,6 @@ repeat: trunc = pflag = 0; switch (cmd) { case '&': - /* This is not working now */ skipblank(); chkprint(0); if (!ocmdline) @@ -1254,8 +1253,6 @@ repeat: execsh(); break; case '\0': - if (gflag && uflag) - return; num = gflag ? curln : curln+1; deflines(num, num); pflag = 'p'; @@ -1520,17 +1517,28 @@ doglobal(void) zero[k].global = 0; curln = ln; nlines = 0; - if (uflag) { - line1 = line2 = ln; - pflag = 0; - doprint(); + + if (!uflag) { + idx = inputidx; + getlst(); + docmd(); + inputidx = idx; + continue; + } + + line1 = line2 = ln; + pflag = 0; + doprint(); + + for (;;) { getinput(); + if (strcmp(cmdline.str, "") == 0) + break; savecmd(); + getlst(); + docmd(); } - idx = inputidx; - getlst(); - docmd(); - inputidx = idx; + } else { cnt++; ln = nextln(ln); -- 2.37.3
[hackers] [PATCH 4/6] ed: Print only last line in empty command
--- ed.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ed.c b/ed.c index 35fddf1..eaa4ca9 100644 --- a/ed.c +++ b/ed.c @@ -1255,6 +1255,7 @@ repeat: case '\0': num = gflag ? curln : curln+1; deflines(num, num); + line1 = line2; pflag = 'p'; goto print; case 'l': -- 2.37.3
[hackers] [PATCH 1/6] ed: Fix makeline
Strings without newlines created problems in the function and the global field was not updated, making that new lines added were marked as global being processed in the current global command. --- ed.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ed.c b/ed.c index a8bc7d5..b6f4f1c 100644 --- a/ed.c +++ b/ed.c @@ -185,19 +185,20 @@ makeline(char *s, int *off) if (lastidx >= idxsize) { lp = NULL; if (idxsize <= SIZE_MAX - NUMLINES) - lp = reallocarray(zero, idxsize + NUMLINES, sizeof(*lp)); + lp = reallocarray(zero, idxsize + NUMLINES, sizeof(*lp)); if (!lp) error("out of memory"); idxsize += NUMLINES; zero = lp; } lp = zero + lastidx; + lp->global = 0; if (!s) { lp->seek = -1; len = 0; } else { - while ((c = *s++) != '\n') + while ((c = *s++) && c != '\n') /* nothing */; len = s - begin; if ((lp->seek = lseek(scratch, 0, SEEK_END)) < 0 || -- 2.37.3
[hackers] [PATCH 0/6] Fix global commands and signal improvements
This patch serie fixes several problems related to how global commands are managed, fixing G and V commands, and it also adds several small improvements in how signals are handled. Roberto E. Vargas Caballero (6): ed: Fix makeline ed: Remove nothing comments ed: Fix G and V commands ed: Print only last line in empty command ed: Simplify sighup dealing ed: Don't undo commands in sigint ed.c | 47 ++- 1 file changed, 30 insertions(+), 17 deletions(-) -- 2.37.3
Re: [hackers] ed: Enable multi line commands
Hi, On Sun, Dec 24, 2023 at 12:07:44PM +0100, Rene Kita wrote: > On Wed, Dec 13, 2023 at 12:55:26PM +0100, Roberto E. Vargas Caballero wrote: > > It changes to read full lines before executing commands, escaping > > newlines when it is needed. It solves 2 different cases: > > > > - Substitution commands with newlines in the replace part > > This does not work with a range: > > #v+ > $ ./ed > i > foobar1 > foobar2 > . > 1,2s/foo/&\ > &/ > ,n > 1 foo > 2 foo > 3 foobar1 > 4 foobar2 Good catch, I am going to add it to the list of known bugs. Substitutions have problems, and this is the next thing that I want to fix. > #v- > > > - Global commands with append or insert commands > > > > Still, some additional problems were detected in the case of > > global commands but they will be fixed in a follow up patchset. > > I did not test much, but what I tested worked. > > BTW, why is there no test suite? I started adding tests[0] for ed when I > attempted to write a patch for this. It's still WIP, though. Basically due to a lack of time. It is something that I discussed with quinq a few weeks ago. We should add a test suite for all the sbase commands, and mainly for ed and sed that at this moment are the most complex one (the coming bc is also complex, but a bit less in my opinion). Regards
Re: [hackers] [PATCH 1/6] ed: Add optional parameter to string()
Hi, On Tue, Dec 26, 2023 at 03:40:36PM +0100, Страхиња Радић wrote: > On 23/12/24 11:46AM, Rene Kita wrote: > > > + if (!from) { > > > + len = 0; > > > + t = NULL; > > > + } else { > > This seems redundant. Normally, NULL shouldn't be passed, and even if it is, > it > is the responsibility of the "user-programmer" (think libc functions). This > is > further backed by the standard pattern of always checking for failed malloc. As Rene commented maybe is better to split this function in two different functions. Assigning NULL in this case was needed because it was used later in a call to realloc. Ed is different to other sbase programs where is acceptable to abort when we run out of memory, because it is mainly used in an interactive way and discarding all the changes from the user in that case seems a bit weird. Regards,
Re: [hackers] [PATCH 1/6] ed: Add optional parameter to string()
Hi, On Sun, Dec 24, 2023 at 11:46:26AM +0100, Rene Kita wrote: > Nit as it's more a matter of style: I'd prefer to have one function to > create a String and another function to create a String from a char > array. This would make a cleaner interface and avoids passing and > dealing with NULL all the time. > This was my election in the first version, but then I moved to unify both. I think I will attend your comment and split it again. I will push it with that small modification and then I will send the second patch serie. Regards,
[hackers] [PATCH 6/6] ed: Update TODO
Remove the cases are tested to work correctly now. --- TODO | 16 1 file changed, 16 deletions(-) diff --git a/TODO b/TODO index a78cf8b..000fd06 100644 --- a/TODO +++ b/TODO @@ -28,10 +28,6 @@ Bugs ed -- -* Multi-line commands don't work in global commands: -g/^line/a \ -line1 -. * cat < - ->bar - ,p - foo - ->bar - Q printf -- 2.37.3
[hackers] [PATCH 5/6] ed: Improve execsh
--- ed.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/ed.c b/ed.c index 16fbe04..60673a2 100644 --- a/ed.c +++ b/ed.c @@ -1061,13 +1061,21 @@ execsh(void) } while ((c = input()) != '\0') { - if (c == '%' && (cmd.siz == 0 || cmd.str[cmd.siz - 1] != '\\')) { + switch (c) { + case '%': if (savfname[0] == '\0') error("no current filename"); repl = 1; for (p = savfname; *p; ++p) addchar(*p, &cmd); - } else { + break; + case '\\': + c = input(); + if (c != '%') { + back(c); + c = '\\'; + } + default: addchar(c, &cmd); } } -- 2.37.3
[hackers] [PATCH 4/6] ed: Avoid dangling pointer in getrhs()
If the string r.str is freed but error() is called then next call will see a pointer that maybe it will try to free because the call to error unwind the frame stack. --- ed.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ed.c b/ed.c index ad6c81a..16fbe04 100644 --- a/ed.c +++ b/ed.c @@ -1096,9 +1096,9 @@ getrhs(int delim) } if (!strcmp("%", s.str)) { - free(s.str); if (!rhs) error("no previous substitution"); + free(s.str); } else { free(rhs); rhs = s.str; -- 2.37.3
[hackers] [PATCH 2/6] ed: Add getinput() and setinput()
These functions allow to read from stdin the full next line or seting as input a character array. These functions avoid all the complexity about repeat commands that is very fragile and depends on having multiple global variables with weak relation between them. --- ed.c | 171 --- 1 file changed, 92 insertions(+), 79 deletions(-) diff --git a/ed.c b/ed.c index 13e956a..7881fba 100644 --- a/ed.c +++ b/ed.c @@ -64,30 +64,15 @@ static int pflag, modflag, uflag, gflag; static size_t csize; static String cmdline; static char *ocmdline; -static int repidx; +static int inputidx; static char *rhs; static char *lastmatch; static struct undo udata; static int newcmd; -int eol, bol; +static int eol, bol; static sig_atomic_t intr, hup; -static void -discard(void) -{ - int c; - - if (repidx >= 0 || cmdline.siz == 0) - return; - - /* discard until the end of the line */ - if (cmdline.str[cmdline.siz-1] != '\n') { - while ((c = getchar()) != '\n' && c != EOF) - ; - } -} - static void undo(void); static void @@ -102,7 +87,6 @@ error(char *msg) if (!newcmd) undo(); - discard(); curln = ocurln; longjmp(savesp, 1); } @@ -166,30 +150,22 @@ static void chksignals(void); static int input(void) { - int c; - - if (repidx >= 0) - return ocmdline[repidx++]; - - if ((c = getchar()) != EOF) - addchar(c, &cmdline); + int ch; chksignals(); - return c; + ch = cmdline.str[inputidx]; + if (ch != '\0') + inputidx++; + return ch; } static int back(int c) { - if (repidx > 0) { - --repidx; - } else { - ungetc(c, stdin); - if (c != EOF) - --cmdline.siz; - } - return c; + if (c == '\0') + return c; + return cmdline.str[--inputidx] = c; } static int @@ -197,7 +173,8 @@ makeline(char *s, int *off) { struct hline *lp; size_t len; - char c, *begin = s; + char *begin = s; + int c; if (lastidx >= idxsize) { lp = NULL; @@ -420,18 +397,14 @@ compile(int delim) eol = bol = bracket = lastre.siz = 0; for (n = 0;; ++n) { - if ((c = input()) == delim && !bracket) + c = input(); + if (c == delim && !bracket || c == '\0') { break; - if (c == '^') { + } else if (c == '^') { bol = 1; } else if (c == '$') { eol = 1; - } else if (c == '\n' || c == EOF) { - back(c); - break; - } - - if (c == '\\') { + } else if (c == '\\') { addchar(c, &lastre); c = input(); } else if (c == '[') { @@ -515,9 +488,8 @@ ensureblank(void) case ' ': case '\t': skipblank(); - case '\n': + case '\0': back(c); - case EOF: break; default: error("unknown command"); @@ -675,6 +647,46 @@ quit(void) exit(exstatus); } +static void +setinput(char *s) +{ + string(&cmdline, s); + inputidx = 0; +} + +static void +getinput(void) +{ + int ch; + + string(&cmdline, NULL); + + while ((ch = getchar()) != '\n' && ch != EOF) { + if (ch == '\\') { + if ((ch = getchar()) == EOF) + break; + if (ch != '\n') { + ungetc(ch, stdin); + ch = '\\'; + } + } + addchar(ch, &cmdline); + } + + addchar('\0', &cmdline); + inputidx = 0; + + if (ch == EOF) { + chksignals(); + if (ferror(stdin)) { + exstatus = 1; + fputs("ed: error reading input\n", stderr); + } + quit(); + } +} + + static void dowrite(const char *, int); static void @@ -866,12 +878,12 @@ chkprint(int flag) else back(c); } - if (input() != '\n') + if (input() != '\0') error("invalid command suffix"); } static char * -getfname(char comm) +getfname(int comm) { int c; char *bp; @@ -879,7 +891,7 @@ getfname(char comm) skipblank(); for (bp = fname; bp < &fname[FILENAME_MAX]; *bp++ = c) { - if ((c = input()) == EOF || c == '\n') + if ((c = input()) == '\0') break; } if (bp == fname) { @@ -1034,7 +1
[hackers] [PATCH 3/6] ed: Read from input in append()
This enables using a and i commands in a global command because the input is not anymore taken from stdin. --- ed.c | 30 ++ 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/ed.c b/ed.c index 7881fba..ad6c81a 100644 --- a/ed.c +++ b/ed.c @@ -686,6 +686,15 @@ getinput(void) } } +static int +moreinput(void) +{ + if (!uflag) + return cmdline.str[inputidx] != '\0'; + + getinput(); + return 1; +} static void dowrite(const char *, int); @@ -870,7 +879,7 @@ dohelp(void) static void chkprint(int flag) { - char c; + int c; if (flag) { if ((c = input()) == 'p' || c == 'l' || c == 'n') @@ -878,7 +887,7 @@ chkprint(int flag) else back(c); } - if (input() != '\0') + if ((c = input()) != '\0' && c != '\n') error("invalid command suffix"); } @@ -913,16 +922,21 @@ getfname(int comm) static void append(int num) { - char *s = NULL; - size_t len = 0; + int ch; + static String line; curln = num; - while (getline(&s, &len, stdin) > 0) { - if (*s == '.' && s[1] == '\n') + while (moreinput()) { + string(&line, NULL); + while ((ch = input()) != '\n' && ch != '\0') + addchar(ch, &line); + addchar('\n', &line); + addchar('\0', &line); + + if (!strcmp(line.str, ".\n") || !strcmp(line.str, ".")) break; - inject(s, AFTER); + inject(line.str, AFTER); } - free(s); } static void -- 2.37.3
[hackers] [PATCH 1/6] ed: Add optional parameter to string()
This makes possible to use the function to initialize the string from an existing char array. --- ed.c | 30 +- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/ed.c b/ed.c index b430e74..13e956a 100644 --- a/ed.c +++ b/ed.c @@ -122,12 +122,24 @@ prevln(int line) } static String * -string(String *s) +string(String *s, char *from) { + size_t len; + char *t; + + if (!from) { + len = 0; + t = NULL; + } else { + if ((t = strdup(from)) == NULL) + error("out of memory"); + len = strlen(t); + } + free(s->str); - s->str = NULL; - s->siz = 0; - s->cap = 0; + s->str = t; + s->siz = len; + s->cap = len; return s; } @@ -949,7 +961,7 @@ join(void) char *t, c; static String s; - string(&s); + string(&s, NULL); for (i = line1;; i = nextln(i)) { chksignals(); for (t = gettxt(i); (c = *t) != '\n'; ++t) @@ -1014,7 +1026,7 @@ execsh(void) skipblank(); if ((c = input()) != '!') { back(c); - string(&cmd); + string(&cmd, NULL); } else if (cmd.siz) { --cmd.siz; repl = 1; @@ -1048,7 +1060,7 @@ getrhs(int delim) int c; static String s; - string(&s); + string(&s, NULL); while ((c = input()) != '\n' && c != EOF && c != delim) addchar(c, &s); addchar('\0', &s); @@ -1152,7 +1164,7 @@ subline(int num, int nth) int i, m, changed; static String s; - string(&s); + string(&s, NULL); i = changed = 0; for (m = match(num); m; m = rematch(num)) { chksignals(); @@ -1456,7 +1468,7 @@ doglobal(void) int cnt, ln, k; skipblank(); - string(&cmdline); + string(&cmdline, NULL); gflag = 1; if (uflag) chkprint(0); -- 2.37.3
[hackers] ed: Enable multi line commands
It changes to read full lines before executing commands, escaping newlines when it is needed. It solves 2 different cases: - Substitution commands with newlines in the replace part - Global commands with append or insert commands Still, some additional problems were detected in the case of global commands but they will be fixed in a follow up patchset.
Re: [hackers] [sbase] sbase-box: Fix segmentation fault when exe without args
Hi, On Fri, Dec 01, 2023 at 01:33:36PM +0100, Jules Maselbas wrote: > when sbase-box is executed without argument, the check sbase-box > options doesn't verify the argument count leading to a segfault. > > Add a check on the argc before parsing sbase-box options (currently > only `-i`) Applied, thanks!
Re: [hackers] [sbase][PATCH] ed: Allow newlines in a Substitute Command
Hi, On Wed, Nov 15, 2023 at 08:56:56AM +0100, Rene Kita wrote: > > I think the way to fix this problem is reading the full command before > > executing it, otherwise there are so many traps. I am going to try to > > fix this in th enext days. > > I had the same idea. Reading the full command will put the > logic to detect the end of the command in one place. This should give > better code. > > Also, you will have to change how addresses for the global command are > collected/remembered. We need to act on the original lines and not on > the lines inserted by a global command. I have sent the patches to the hacker ml. I tested them for a few days and it seems to work fine, but would be wonderful if you can take a look and see if they are fine for you too. Regards,
Re: [hackers] [sbase][PATCH] ed: Allow newlines in a Substitute Command
Hi, On Sun, Nov 05, 2023 at 02:38:20PM +0100, Rene Kita wrote: > On Fri, Nov 03, 2023 at 01:32:37PM +0100, Rene Kita wrote: > > borked patch > > Patch is not sufficient, sorry for the noise. I have this problem in my radar. I began to write a solution for it, but I had to switch to implement bc(1) that I hope I will submit soon. I think the way to fix this problem is reading the full command before executing it, otherwise there are so many traps. I am going to try to fix this in th enext days. Regards,
Re: [hackers] [sbase] TODO: add replacement bug reported for ed || Hiltjo Posthuma
Applied, thanks.
Re: [hackers] [PATCH] find: Make parameter error messages more specific
Applied, thanks.
Re: [hackers] [PATCH 2/2] scripts: Fix non-portable find -perm /mode
Applied, thanks.
Re: [hackers] [PATCH 1/2] scripts: Fix non-portable usage of find -maxdepth
Applied, thanks.
Re: [hackers] [PATCH] scripts: Force file copying on install
Applied, thanks.
Re: [hackers] [PATCH] make: fix rogue parameter in install target
Applied, thanks.
Re: [hackers] [sbase][PATCH] Ensure commands are followed by a blank
Applied.
Re: [hackers] [sbase][PATCH] xargs: implement -I flag
Hi, On Sun, Jul 30, 2023 at 10:15:49AM +0300, sewn wrote: > From 9f4be567ff25fee986976c6afa193223496013a6 Mon Sep 17 00:00:00 2001 > From: sewn > Date: Fri, 28 Jul 2023 18:58:37 +0300 > Subject: [PATCH] xargs: add replace string flag (-I) I have applied the patch with some small modifications that I am going to elaborate in the mail. There is still pending work to have a correct -I implementation. > diff --git a/libutil/strnsubst.c b/libutil/strnsubst.c > new file mode 100644 > index 000..a76d484 > --- /dev/null > +++ b/libutil/strnsubst.c > @@ -0,0 +1,63 @@ > +/* > + * Copyright (c) 2002 J. Mallett. All rights reserved. > + * You may do whatever you want with this file as long as > + * the above copyright and this notice remain intact, along > + * with the following statement: > + * For the man who taught me vi, and who got too old, too young. > + */ > + > +#include > +#include > +#include > +#include unistd.h is not needed and it was removed. > +void > +strnsubst(char **str, const char *match, const char *replstr, size_t maxsize) > +{ > + char *s1, *s2, *this; > + size_t matchlen, repllen, s2len; > + int n; replen was set but it was not used, thus I remove it. > +done: > + *str = s2; *str is holding a previously allocated string, so we have to free it. > diff --git a/util.h b/util.h > index 8d5004b..d6911bd 100644 > --- a/util.h > +++ b/util.h > @@ -63,6 +63,8 @@ size_t estrlcpy(char *, const char *, size_t); > #define strsep xstrsep > char *strsep(char **, const char *); > > +void strnsubst(char **str, const char *match, const char *replstr, size_t > maxsize); > + We don't use parameter names in prototypes. > @@ -252,7 +260,11 @@ main(int argc, char *argv[]) > leftover = 1; > break; > } > - cmd[i] = estrdup(arg); > + if (ri > 0) > + strnsubst(&cmd[ri], replstr, arg, (size_t)255); > + else > + cmd[i] = estrdup(arg); > + > argsz += arglen + 1; > i++; > a++; the cast to size_t is not required because the prototype already does the work. The problems with this implementation is that POSIX mandates that when -I is used then spaces don't break arguments and they are just separated by newlines. I tried something like: - case ' ': case '\t': case '\n': + case ' ': + case '\t': + if (Iflag) + goto fill; + case '\n': goto out; case '\'': if (parsequote('\'') < 0) @@ -126,6 +130,7 @@ poparg(void) eprintf("backslash at EOF\n"); break; default: + fill: fillargbuf(ch); argbpos++; break; @@ -227,6 +232,7 @@ main(int argc, char *argv[]) eofstr = EARGF(usage()); break; case 'I': + Iflag = 1; xflag = 1; nflag = 1; maxargs = 1; but we pass the full line (after removing leading spaces) to exec, being a single parameter instead of being split. For example: xargs -I x ls x <
Re: [hackers] [ubase][PATCH] Explicitly include sys/sysmacros.h for makedev etc
On Sun, Aug 06, 2023 at 03:03:21PM +0200, Markus Rudy wrote: > This header used to be included by sys/types.h in glibc, and musl > adopted the behaviour. However, this dependency was never desired, so > glibc deprecated it in 2016 and finally removed it in 2019, and so Applied.
Re: [hackers] [sbase][PATCH] tr: fix behavior of cflag when using character classes
Hi, On Sun, Aug 06, 2023 at 10:50:25PM +0200, noneofyourbusin...@danwin1210.de wrote: > a simple test case: > > printf ab3 | tr -c '[:alpha:]' '\n' Applied.
Re: [hackers] [st][PATCH] Add support for DSR response "OK" escape sequence
On Tue, Feb 07, 2023 at 10:54:57AM -0500, Adam Price wrote: > On Mon, Feb 6, 2023 at 12:06 PM Roberto E. Vargas Caballero > wrote: > > > > Hi, > > > > On Mon, Feb 06, 2023 at 08:45:27AM +0200, Santtu Lakkala wrote: > > > > tsetattr(csiescseq.arg, csiescseq.narg); > > > > break; > > > > - case 'n': /* DSR – Device Status Report (cursor position) */ > > > > - if (csiescseq.arg[0] == 6) { > > > > + case 'n': /* DSR – Device Status Report */ > > > > + switch (csiescseq.arg[0]) { > > > > + case 5: /* Status Report "OK" `0n` */ > > > > + ttywrite("\033[0n", sizeof("\033[0n"), 0); > > > > > > This will write a NUL byte to the tty, which doesn't seem intentional. > > > > Indeed, but it should not have any difference because '\0' is a control > > character that in this situation is ignored by the terminal. Anyway it > > should be avoided. > > Ah right, of course. Thank you to you two for pointing that out. I should use > strlen() instead of sizeof(). > > I will send an updated patch here shortly. > No, sizeof()-1. There is no reason to call strlen when you know the size. Regards.
Re: [hackers] [st][PATCH] Add support for DSR response "OK" escape sequence
Hi, On Mon, Feb 06, 2023 at 08:45:27AM +0200, Santtu Lakkala wrote: > > tsetattr(csiescseq.arg, csiescseq.narg); > > break; > > - case 'n': /* DSR – Device Status Report (cursor position) */ > > - if (csiescseq.arg[0] == 6) { > > + case 'n': /* DSR – Device Status Report */ > > + switch (csiescseq.arg[0]) { > > + case 5: /* Status Report "OK" `0n` */ > > + ttywrite("\033[0n", sizeof("\033[0n"), 0); > > This will write a NUL byte to the tty, which doesn't seem intentional. Indeed, but it should not have any difference because '\0' is a control character that in this situation is ignored by the terminal. Anyway it should be avoided. Regards,
Re: [hackers] [sbase] [PATCH] Use ar(1)'s s-flag instead of invoking ranlib(1)
Hi, On Sun, Jul 31, 2022 at 01:00:25AM +0200, Laslo Hunhold wrote: > why would it reduce the portability of the Makefiles? It can be > expected that all ar-implementations support the s-flag, and ranlib is > simply legacy. Because then you will support only the last systems. If you keep the ranlib you will support systems that support all versions of the standard. Again, if you find a system without ranlib then we can talk and consider what to do, but removing only for the sake of "the standard does not include anymore ranlib" is a horrible idea. For example, scc requires the use of ranlib, if you remove it then I will not be able to continue testing scc with suckless software. What happens if I want to compile sbase in an old SunOs workstation? Please, keep busy doing actual work and stop doing meaningless changes. Roberto.
Re: [hackers] [dwm] Revert "do not call signal-unsafe function inside sighanlder" || Hiltjo Posthuma
Hi, > > void > > sigchld(int unused) > > { > > + if (signal(SIGCHLD, sigchld) == SIG_ERR) > > + die("can't install SIGCHLD handler:"); > > while (0 < waitpid(-1, NULL, WNOHANG)); > > } > > Calling `die` inside a signhandler is still an issue and can produce > bad behavior (I have seen a couple software which randomly freeze due to > calling signal unsafe functions inside a sighandler). > > If `sigaction` can fix the issue with `signal` then it's probably best > to use that. Otherwise replacing `die` with `write` and `_exit` is also > a viable solution as shown in: > https://lists.suckless.org/hackers/2207/18405.html Indeed. in both things, we should not use die() in a signal handler and using signal to reinstall the handler opens a race condition window where we can lose sigchld signals and generate zombies process. I personally think that using a sigchld handler to collect zomby process is a bad idea. I think it is POSIX that if you use an explicit signal call to ignore SIGCHLD then the system reap them automatically without explicits wait [1]. I could not find this specification in the POSIX standard, but [2] says: POSIX.1-2001 specifies that if the disposition of SIGCHLD is set to SIG_IGN or the SA_NOCLDWAIT flag is set for SIGCHLD (see sigaction(2)), then children that terminate do not become zombies and a call to wait() or waitpid() will block until all children have terminated, and then fail with errno set to ECHILD. So it seems it is standard since long time ago. [1] https://copyprogramming.com/howto/what-is-the-use-of-ignoring-sigchld-signal-with-sigaction-2 [2] https://corp.sonic.net/ceo/perl-sigchld-ignore-system-and-you/ Roberto Vargas,
Re: [hackers] [dwm][PATCH] do not call signal-unsafe function inside sighanlder
Hi, > > Do you have a reference of a description of this behaviour in an other > > system, > > specification or standard? > > > > C89 (7.7.1.1), C99 (7.14.1.1), POSIX 2001 and 2008 all say that "the > equivalent of signal(sig, SIG_DFL)" may be executed prior to executing > the signal handler (but is not required as long as the signal is blocked > until the handler has returned). > > As for implementations, my signal(2) man page says BSD and glibc systems > do not reset the handler, but the linux syscall does, as well as > "original UNIX systems" and System V. I've no idea what other libcs do. > Yes, this is the problem with signal(3) and why the sigaction(2) interface was designed (they didn't want to enforce bsd or systemV, and they just created a new (and more complex) interface. If you use signal() you cannot assume any behaviour. Also, notice that the bsd behaviour broke alarm() because system calls are always restarted instead of generating EINTR. Regards,
Re: [hackers] [sbase] [PATCH] Use ar(1)'s s-flag instead of invoking ranlib(1)
I disagree with this change. I think it adds nothing and reduce portability of the Makefiles. Regards,
Re: [hackers] [st][PATCH] update FAQ regarding meta key
Hi, > just changing $TERM to "st-meta" doesn't enable the meta key, at least > on vim. searching the mailing list, I learned that `tput smm` was needed > to enable 8bit mode[0]. This topic is a bit more complex. St is doing something a bit weird because we are a utf8 terminal, but we don't encode an utf8 sequence for the meta key. I talked about this topic with Thomas E. Dickey, the maintainer of xterm and ncurses (including terminfo) and he confirmed that we are doing it worng. This is the reason why he didn't add the st-meta definition in the official terminfo distribution. To be a correct utf8 terminal with meta key we should generate a utf8 sequence for the meta combination. For example, Meta-A generates 0xC1 that enconded in utf8 must be 0xC3 0x81. I don't know if it is worth to try to fix this or not. Regards,
Re: [hackers] [st][PATCH v2] code-golfing: cleanup osc color related code
Hi, A few small nitpicks about formating (fell free to ignore them if you want ;)): On Sun, Mar 20, 2022 at 06:25:40PM +0600, NRK wrote: > @@ -1843,39 +1844,25 @@ csireset(void) > } > > void > -osc4_color_response(int num) > +osc_color_response(int num, int index, int is_osc4) > { > int n; > char buf[32]; > unsigned char r, g, b; > > - if (xgetcolor(num, &r, &g, &b)) { > - fprintf(stderr, "erresc: failed to fetch osc4 color %d\n", num); > + if (xgetcolor(is_osc4 ? num : index, &r, &g, &b)) { > + fprintf(stderr, "erresc: failed to fetch %s color %d\n", > + is_osc4 ? "osc4" : "osc", is_osc4 ? num : index); I think is better to keep every ternary in a line by itself, because it makes easier to read them: fprintf(stderr, "erresc: failed to fetch %s color %d\n", is_osc4 ? "osc4" : "osc", is_osc4 ? num : index); > + n = snprintf(buf, sizeof buf, > "\033]%s%d;rgb:%02x%02x/%02x%02x/%02x%02x\007", > + is_osc4 ? "4;" : "", num, r, r, g, g, b, b); > + if (n < 0 || n >= sizeof(buf)) > + fprintf(stderr, "error: %s while printing %s response\n", n < 0 > ? > + "snprintf failed" : "truncation occurred", is_osc4 ? > "osc4" : "osc"); > + else > + ttywrite(buf, n, 1); > } I think we force here to put braces around if and else because the body of the if part is more of one line. Again, I think is better to use a line for every ternary and have something like: if (n < 0 || n >= sizeof(buf)) { fprintf(stderr, "error: %s while printing %s response\n", n < 0 ? "snprintf failed" : "truncation occurred", is_osc4 ? "osc4" : "osc"); } else { ttywrite(buf, n, 1); } > + if ((j = par - 10) < 0 || j >= LEN(osc_table)) > + break; /* shouldn't be possible */ > > if (!strcmp(p, "?")) > - osc_color_response(defaultcs, 12); > - else if (xsetcolorname(defaultcs, p)) > - fprintf(stderr, "erresc: invalid cursor color: > %s\n", p); > + osc_color_response(par, osc_table[j].idx, 0); > + else if (xsetcolorname(osc_table[j].idx, p)) > + fprintf(stderr, "erresc: invalid %s color: > %s\n", > + osc_table[j].str, p); > else > tfulldirt(); > return; Same apply here, I think our style forces to have braces in every if and else if because there is a body with more of one lines. Regards,
Re: [hackers] [st][PATCH] rm unnecessary explicit zeroing
Hi, On Fri, Mar 18, 2022 at 03:16:56AM +0600, NRK wrote: > But yes, you're right, you'd need 256 elements to be able to index into > an array as any unsigned char. So maybe it *should* be 256. Uh, I didn't realize about it, I just saw that having 255 entries was wrong ^^!!!. I think the best approach is to split the commit in two and evaluate/review them isolated; one commit to fix the size, and other about zeroing. Regards,
Re: [hackers] [st][PATCH] rm unnecessary explicit zeroing
Hi, On Tue, Mar 15, 2022 at 04:30:52PM +0600, NRK wrote: > +static const char base64_digits[(unsigned char)-1] = { Any reason to write "(unsigned char)-1" instead of writing 256? Regards,
Re: [hackers] st][PATCH - proper escape sequence for CTRL+HOME
Hi, On Tue, Mar 01, 2022 at 08:54:15AM -0500, Sebastian LaVine wrote: > > Christ, why do you choose to be so rude to someone you've never talked > to over a simple email? He wants to write comments for a C program. It's > not the end of the world. I personally will be interested in what he > does; he isn't wrong that a lot of the main suckless code isn't well > commented. You really think there's something wrong with "people like > [him]" studying something he doesn't understand, and offering to share > the results of his study with others, so that they too might have a > better understanding? Weird. Drink some tea and calm down. > Maybe we have different cultures, but my impression is that who was rude was him. Sending a mail saying "you bad, I good, do what I tell you" is VERY rude where I come from. We don't want people here with those comments. If you want to send patches, send them otherwise just shut up. There are a lot of ways of thinking and we don't want to force anyone to change their way of thinking, but please don'try to do the same with us, or at least be polite. I talked with other developers and they also were offended. He did a really bad comment and he mistook in his conclusions. The objective of comments in St code is not to explain how a terminal must work, there are plenty of documents explaining that. Do you imagine the code of the linux kernel full of comments explaining how an operating system must work? Do you have comments in man pages explaining what is a file descriptor in evey man page? You already have books for that. If he had problems understanding something he could just ask, not sending an arrrogant mail menacing others with the option of forking the code. We don't want that here, if you like the code contribute, if you don't like it contribute to do it better, but don't complain if your patches are not accepted. Regards,
Re: [hackers] st][PATCH - proper escape sequence for CTRL+HOME
Hi, On Mon, Feb 28, 2022 at 09:27:22PM -0600, Dave Blanchard wrote: > > I have absolutely no idea what the 'appkey' and 'appcursor' fields do, as > there are almost no comments anywhere to be found in the source code, and I > haven't yet reverse engineered the code enough to figure out what the hell > it's actually doing with those values. The provided values seem to work fine, > though they may need to be changed if they're wrong. > 'appkey' and 'appcursor' are modes in the vt100, you only have to search in the vt100 documentation [1] for application mode and/or numeric mode. You can also search for the terminfo sequences rmkx and smkx in terminfo(5). > On that note, regrettably it will be necessary for me to fork this project, > if for no other reason than to properly comment it, so that its functionality > can be understood and easily modified. It's a shame that such a nice little > program is marred by its total lack of commentation, along with poorly chosen > function and variable names. The use of tabs in the source code isn't > particularly desirable either, IMO. Please, fork it and leave us quiet, we don't need people like you that are proud of not knowing things. Appkey and Appcursor are related to the vt100 way of working, and the objective of the source code of st is not to teach you about how terminals work. You should learn that by yourself. As, you demostrated your zero capability to search for documentation I give you a link [2] that explains the topic. Please, the next time before going into a community with that total lack of etiquette try to search information about the topic, because you were requesting info for something that was not part of the structure of the code itself. Regards, [1] https://vt100.net/docs/vt100-ug/ [2] https://ttssh2.osdn.jp/manual/4/en/usage/tips/appkeypad.html
Re: [hackers] Re: [ubase] [PATCH] Include sys/sysmacros.h when major is not defined in sys/types.h
On Mon, Nov 04, 2019 at 08:01:18AM +0100, Laslo Hunhold wrote: > Dimitris is the current maintainer, so you will have to talk to him, > but I'd say that nothing speaks against you maintaining it. I always > saw sbase and ubase to be siblings, so given you already maintain > sbase, it would make a lot of sense. :) He is in the mailing list, but I think he usually doesn't read it. Maybe a direct mail is better. Regards,
Re: [hackers] [st][patch] Increase the buffer size for escape sequences
On Mon, Sep 24, 2018 at 05:45:29PM -0700, Eric Pruitt wrote: > I agree that the current buffer is too small. I'm pretty sure I've run > into this problem myself with Vim and Bash, but I hadn't gotten around > to digging into the problem. If we go to increase that size, I would go to use dynamic memory. Having an array of 1MB statically allocated is a crazy idea (and it is not C99 compliant, where the maximun allocated size is 128K).
Re: [hackers] [st][patch] Increase the buffer size for escape sequences
On Sun, Sep 23, 2018 at 03:56:43PM +0200, Ingo Heimbach wrote: > What is incorrect? I would say, why 1048576 and not 1000? or 1?. Is there a specific reason? Roberto
Re: [hackers] [ii][patch] add support for OpenBSD unveil(2)
Hi, On Wed, Sep 12, 2018 at 08:08:39PM +0200, Laslo Hunhold wrote: > that's your choice as the maintainer and I am not a fanboy. OpenBSD is > objectively more secure and it's mainly due to their approach. Credit > where credit is due. You shpuld read those [1] and [2]. OpenBSD *IS NOT* objectively more secure. It only had less security defects because it has less people inspecting the code. For so many years OpenBSD was running with very important vulnerabilities that weren't noticied by anyone. > > If you don't understand any of my reasons, then you should stop > > posting here and begin to post to OpenBSD, I am pretty sure that Theo > > will be more friendly than we are (irony mode off). > > Your reasons are simple to understand. The main argument is to > ask: "When we add OpenBSD-specific code, why not Linux-specific code as > well?". No, my point is about having suckless code, and having that ifdef there makes the code suckmore. Offline I suggested other solutions, as Dimitris and Hiltjo can confirm, like for example having the patches in the repo and a rule in the Makefile to patch the sources, or like creating local versions of the interfaces (ex: mypledge) and having the ifdef there, or having a file per system with the specific code of the system. All this options were discarded because at the end we are missing the point of suckless: Good code and simplicity as first objective. > In an ideal world we would have portable interfaces for this, but there > aren't. Surely ii runs without unveil() just fine, however, you have > bigger problems when you need a good source of entropy that is secure > to "tap". No. This is how when we complaint about the linux users putting #/bin/bash or using GNU extensions in Makefiles. Core OpenBSD developers are totally differtent, but OpenBSD is creating a full culture of people around that only has a centralized view of the world. They don't contrast the point and they don't generate a critical actitude, everything that comes from OpenBSD is right, and OpenBSD is the more secure system, which is obviously false (there are other systems that are more secure and more reliable, but maybe less usable, than OpenBSD). This is why I called you a fanboy, because you don't have that critical spirit and you don't try to think by yourself, you only repeat dogmas that someone else created. Roberto. [1] https://www.openbsd.org/papers/fuzz-slides.pdf [2] https://media.defcon.org/DEF%20CON%2025/DEF%20CON%2025%20presentations/DEFCON-25-Ilja-van-Sprundel-BSD-Kern-Vulns.pdf
Re: [hackers] [ii][patch] add support for OpenBSD unveil(2)
On Wed, Sep 12, 2018 at 10:19:32AM +0200, Laslo Hunhold wrote: > Adding ifdefs of course is a tough decision in any case, though I > always think that suckless tools should be really more tuned towards > OpenBSD as it really is probably the most suckless operating system > around. You are wrong, there is nothing about OpenBSD in suckless. You can write suckless code in Windows, and any unix alike operating system today sucks a lot. > > If we turn this into patches it just means more work in maintenance > and, as quoted above, optional security is often forgotten. Also, this > change is relatively simple and we don't have an ifdef-tree or anything. Your oppinion is irrelevant, I don't accept sugestions form fanboys. This is not about security, it is about writing suckless code that can be understood easily, that can be maintained easily and it is portable. Security is about designing good system and doing a proper separation of responsabilities. Mitigations are only a distraction. You should read [1]. If you don't understand any of my reasons, then you should stop posting here and begin to post to OpenBSD, I am pretty sure that Theo will be more friendly than we are (irony mode off). Regards, [1] https://cr.yp.to/qmail/qmailsec-20071101.pdf
Re: [hackers] [ii][patch] add support for OpenBSD unveil(2)
On Tue, Sep 11, 2018 at 08:14:25PM -0300, Gleydson Soares wrote: > the following patch brings support for OpenBSD's unveil(2) mechanism for > ii. Guys, we should stop sending this kind of patches. If we begin to fill all the suckless projects with #ifdef __OpenBSD__, why do we not fill them with #ifdef __linux__? If this patch is accepted then I will send a patch to add suppport for selinux and other one to add support for capsicum. All these patches must go to the patches section and not in the main repo. Regards,
Re: [hackers] [sbase][PATCH v2] Add tests for some utilities
Hi, On Wed, Aug 01, 2018 at 09:16:26PM +0200, Silvan Jegen wrote: > > * `echo` is unportable and `printf` should be used instead. > > Didn't know that echo was not portable. Thought it was just a builtin > that should work the same everywhere. It's probably the flags that are > the issue... echo is portable, bad usage of echo is not portable. Use printf when you want complex numeric conversions or escape sequences, otherwise use echo. > If we go with option 1) I would like to wait to see which C functionality > we would end up needing in the end. Looking at the C code I would postpone > until after that decision has been made. You can do whatever you want, but you should ask to the maintainers first, and I am pretty sure they don't agree with the kind of tests that you want to implement, so don't lose your time and try to have a conversation first with the maintainers. Everything else that is not agreed with them is not going to be applied. Regards,
Re: [hackers] [sbase][PATCH v2] Add tests for some utilities
Hi, On Wed, Aug 01, 2018 at 04:36:35PM +0200, Silvan Jegen wrote: > I definitely think we should have unit tests for sbase (and other > projects?) as soon as possible. What concerns me with your approach is > that we have about 700 lines of C code in testing-common.{c,h} of which > I feel quite a bit could be dropped. No, unit test no.What sbase needs is functional tests. Take a look to [1], [2] and [3]. You don't need anything else. > > I have written some (crappy and probably non-portable) shell script > functions to check the stdout and stderr of a process. It's about 40 > lines. I also converted your tests for dirname to use these functions > (both files attached. The test coverage is not exactly the same but > relatively similar). They are totally bloated. You don't need a framework, you only need a shell script that returns success or fail for every test. and after that, you only have to run over all the test files in the directory. > What do you think? Totally bloated. If you want, you can take a look to the full test structure in scc. Regards. [1] https://git.simple-cc.org/scc/file/tests/ar/execute/0001-append.sh.html [2] https://git.simple-cc.org/scc/file/tests/ar/execute/chktest.sh.html [3] https://git.simple-cc.org/scc/file/tests/ar/execute/Makefile.html
[hackers] [sbase] Improve doglobal()
Don't use directly the line numbers and call to getlst() when a line is matched. --- ed.c | 30 ++ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/ed.c b/ed.c index 13c41c6..e6d92e2 100644 --- a/ed.c +++ b/ed.c @@ -1318,7 +1318,7 @@ chkglobal(void) static void doglobal(void) { - int i, k; + int cnt, ln, k; skipblank(); cmdline.siz = 0; @@ -1326,18 +1326,24 @@ doglobal(void) if (uflag) chkprint(0); - for (i = 1; i <= lastln; i++) { - k = getindex(i); - if (!zero[k].global) - continue; - curln = i; - nlines = 0; - if (uflag) { - line1 = line2 = i; - pflag = 0; - doprint(); + ln = line1; + for (cnt = 0; cnt < lastln;) { + k = getindex(ln); + if (zero[k].global) { + zero[k].global = 0; + curln = ln; + nlines = 0; + if (uflag) { + line1 = line2 = ln; + pflag = 0; + doprint(); + } + getlst(); + docmd(); + } else { + cnt++; + ln = nextln(ln); } - docmd(); } discard(); /* cover the case of not matching anything */ } -- 2.16.2
Re: [hackers] [PATCH] Whitelist key event modifiers for shortcuts
> I'm seeing a weird issue with my xserver where all key press events will > be set with (state & Button1Mask), which ends up breaking all st > keyboard shortcuts. xterm works correctly because it whitelists > modifiers relevant to key press events. Do the same in st. Uhmmm, it seems a bit strange. Why does your xserver do that?. I think your patch isn't wrong, but I think what you describe shouldn't happen, and I would like to know a bit about it before applyin the patch. Regards,
Re: [hackers] [farbfeld] Overhaul Build-system || Laslo Hunhold
>> +png2ff ff2png: LDFLAGS += -lpng >> +jpg2ff ff2jpg: LDFLAGS += -ljpeg >> > > This is invalid and breaks on OpenBSD (and other non-GNU make probably). It is not POSIX, so it is a syntax error for me.
Re: [hackers] [dwm][PATCH] Fix signal race condition
> If the signal(2) call within the signal handler fails, die() is called > which in turn is not signal-safe. Therefore, the change to sigaction > makes dwm() more portable among POSIX systems and fixes a signal race > condition. You are right with the original race condition, but I think your solution has also a race condition, because you are not blocking signals while you are in the handler. From my point of view what should be done here is simply: signal(SIGCHLD, SIG_IGN); It will avoid zombie children. Regards,
Re: [hackers] [sbase] Revert "ed: remove double free in join()" ||
Hello Thomas, Sorry for the delay, I had some problems with my mail lately, > The trouble with reverting my commit is that readding the double free > completely > crashes ed if more than one join is performed. I think this patch (which also > reverts > back to having no double free) should handle your concern via blocking signal > handling > until the join is finished. Ok, I understand. Mixing longjmp signals and dynamic memory was a really bad idea. Your idea is not bad, but I think we should block sigprocmask and related functions, instead of playing with variables. Regards,
Re: [hackers] [sbase] ed: Don't use strlcpy() || Roberto E. Vargas
> I consider this way of thinking harmful, because it involves You are considered harmful. Regards,
[hackers] [sbase][PATCH] [ed] Do not try to rematch patterns with ^ or $
It is impossible to rematch a pattern which has one (or both) of these operators, so the simplest solucion is detect them while we are compiling the regular expression and break the match loop after the first iteration. --- ed.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/ed.c b/ed.c index 9dc6fda..0ec0574 100644 --- a/ed.c +++ b/ed.c @@ -63,6 +63,7 @@ static char *rhs; static char *lastmatch; static struct undo udata; static int newcmd; +int eol, bol; @@ -360,11 +361,15 @@ compile(int delim) if (!isgraph(delim)) error("invalid pattern delimiter"); - bracket = siz = 0; + eol = bol = bracket = siz = 0; for (n = 0;; ++n) { if ((c = input()) == delim && !bracket) break; - if (c == '\n' || c == EOF) { + if (c == '^') { + bol = 1; + } else if (c == '$') { + eol = 1; + } else if (c == '\n' || c == EOF) { back(c); break; } @@ -1007,9 +1012,11 @@ subline(int num, int nth) static size_t siz, cap; i = changed = siz = 0; - for (m = match(num); m && *lastmatch != '\n'; m = rematch(num)) { + for (m = match(num); m; m = rematch(num)) { addpre(&s, &cap, &siz); changed |= addsub(&s, &cap, &siz, nth, ++i); + if (eol || bol) + break; } if (!changed) return; -- 2.1.4
[hackers] [sbase][PATCH] Stop matching when lastmatch points to '\n'
This situation happens with something like s/$/test/, where rm_so == rm_eo == 0. Without this check, ed keeps looping forever. --- ed.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ed.c b/ed.c index 69c7422..9dc6fda 100644 --- a/ed.c +++ b/ed.c @@ -1007,7 +1007,7 @@ subline(int num, int nth) static size_t siz, cap; i = changed = siz = 0; - for (m = match(num); m; m = rematch(num)) { + for (m = match(num); m && *lastmatch != '\n'; m = rematch(num)) { addpre(&s, &cap, &siz); changed |= addsub(&s, &cap, &siz, nth, ++i); } -- 2.1.4
[hackers] [sbase][PATCH v2] Fix pattern substitution
Ed was falling doing substitution different of the first or all (s//%/, s//%/\1, s//%/g), because it was not adding the matches which were not going to be substituted. --- ed.c | 26 +++--- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/ed.c b/ed.c index 77aad19..69c7422 100644 --- a/ed.c +++ b/ed.c @@ -959,12 +959,20 @@ addpost(char **s, size_t *cap, size_t *siz) *s = addchar('\0', *s, cap, siz); } -static void -addsub(char **s, size_t *cap, size_t *siz) +static int +addsub(char **s, size_t *cap, size_t *siz, int nth, int nmatch) { char *end, *q, *p, c; int sub; + if (nth != nmatch && nth != -1) { + q = lastmatch + matchs[0].rm_so; + end = lastmatch + matchs[0].rm_eo; + while (q < end) + *s = addchar(*q++, *s, cap, siz); + return 0; + } + for (p = rhs; (c = *p); ++p) { switch (c) { case '&': @@ -972,7 +980,7 @@ addsub(char **s, size_t *cap, size_t *siz) goto copy_match; case '\\': if ((c = *++p) == '\0') - return; + return 1; if (!isdigit(c)) goto copy_char; sub = c - '0'; @@ -988,24 +996,20 @@ addsub(char **s, size_t *cap, size_t *siz) break; } } + return 1; } static void subline(int num, int nth) { - int m, changed; + int i, m, changed; static char *s; static size_t siz, cap; - siz = 0; + i = changed = siz = 0; for (m = match(num); m; m = rematch(num)) { addpre(&s, &cap, &siz); - if (--nth > 0) - continue; - changed = 1; - addsub(&s, &cap, &siz); - if (nth == 0) - break; + changed |= addsub(&s, &cap, &siz, nth, ++i); } if (!changed) return; -- 2.1.4
[hackers] [sbase][PATCH v2] Handle explicitly the case of line 0
Line 0 is a special line added to allow operations with empty buffers, and we have to ensure that it is not going to match any regular expression. The code was written in a way that this case was handle implicitily, but this solution was working only for the first file loaded in ed, while the second file loaded in ed got a line with a dirty seek field. This solution check explicitily against invalid lines passed to makeline(), which allows to simplify the common case. --- ed.c | 31 +-- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/ed.c b/ed.c index 686d2aa..c560d1a 100644 --- a/ed.c +++ b/ed.c @@ -169,20 +169,20 @@ makeline(char *s, int *off) } lp = zero + lastidx; - while ((c = *s) && *s != '\n') - ++s; - if (c == '\n') - ++s; - len = s - begin; + if (!s) { + lp->seek = -1; + len = 0; + } else { + while ((c = *s++) != '\n') + /* nothing */; + len = s - begin; + if ((lp->seek = lseek(scratch, 0, SEEK_END)) < 0 || + write(scratch, begin, len) < 0) { + error("input/output error"); + } + } if (off) *off = len; - - if (len > 0) - if ((lp->seek = lseek(scratch, 0, SEEK_END)) < 0 || - write(scratch, begin, len) < 0) { - error("input/output error"); - } - ++lastidx; return lp - zero; } @@ -210,8 +210,11 @@ gettxt(int line) char *p; lp = zero + getindex(line); - off = lp->seek; sizetxt = 0; + off = lp->seek; + + if (off == (off_t) -1) + return text = addchar('\0', text, &memtxt, &sizetxt); repeat: if (!csize || off < lasto || off - lasto >= csize) { @@ -341,7 +344,7 @@ setscratch() error("scratch filename too long"); if ((scratch = mkstemp(tmpname)) < 0) error("failed to create scratch file"); - if ((k = makeline("", NULL))) + if ((k = makeline(NULL, NULL))) error("input/output error in scratch file"); relink(k, k, k, k); clearundo(); -- 2.1.4
[hackers] [sbase][PATCH] Handle explicitly the case of line 0
Line 0 is a special line added to allow operations with empty buffers, and we have to ensure that it is not going to match any regular expression. The code was written in a way that this case was handle implicitily, but this solution was working only for the first file loaded in ed, while the second file loaded in ed got a line with a dirty seek field. This solution check explicitily against invalid lines passed to makeline(), which allows to simplify the common case. --- ed.c | 31 +-- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/ed.c b/ed.c index 686d2aa..09eadd9 100644 --- a/ed.c +++ b/ed.c @@ -169,20 +169,20 @@ makeline(char *s, int *off) } lp = zero + lastidx; - while ((c = *s) && *s != '\n') - ++s; - if (c == '\n') - ++s; - len = s - begin; + if (!s) { + lp->seek = -1; + len = 0; + } else { + while ((c = *s) != '\n') + ++s; + len = s - begin; + if ((lp->seek = lseek(scratch, 0, SEEK_END)) < 0 || + write(scratch, begin, len) < 0) { + error("input/output error"); + } + } if (off) *off = len; - - if (len > 0) - if ((lp->seek = lseek(scratch, 0, SEEK_END)) < 0 || - write(scratch, begin, len) < 0) { - error("input/output error"); - } - ++lastidx; return lp - zero; } @@ -210,8 +210,11 @@ gettxt(int line) char *p; lp = zero + getindex(line); - off = lp->seek; sizetxt = 0; + off = lp->seek; + + if (off == (off_t) -1) + return text = addchar('\0', text, &memtxt, &sizetxt); repeat: if (!csize || off < lasto || off - lasto >= csize) { @@ -341,7 +344,7 @@ setscratch() error("scratch filename too long"); if ((scratch = mkstemp(tmpname)) < 0) error("failed to create scratch file"); - if ((k = makeline("", NULL))) + if ((k = makeline(NULL, NULL))) error("input/output error in scratch file"); relink(k, k, k, k); clearundo(); -- 2.1.4
[hackers] [sbase][PATCH] Add egrep and fgrep
These tools are not part of POSIX, but they were part of the original UNIX and even today they are still wide used. The work done by this tools can be done by grep, so this implementation is only masking the code with different names to get the work done. --- Makefile | 4 +++- grep.c | 5 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 1c09cac..f6f8dc8 100644 --- a/Makefile +++ b/Makefile @@ -196,7 +196,9 @@ confstr_l.h limits_l.h sysconf_l.h pathconf_l.h: getconf.sh install: all mkdir -p $(DESTDIR)$(PREFIX)/bin cp -f $(BIN) $(DESTDIR)$(PREFIX)/bin - cd $(DESTDIR)$(PREFIX)/bin && ln -f test [ && chmod 755 $(BIN) + ln $(DESTDIR)$(PREFIX)/bin/grep $(DESTDIR)$(PREFIX)/bin/egrep + ln $(DESTDIR)$(PREFIX)/bin/grep $(DESTDIR)$(PREFIX)/bin/fgrep + cd $(DESTDIR)$(PREFIX)/bin && ln -f test [ && chmod 755 $(BIN) egrep fgrep mkdir -p $(DESTDIR)$(MANPREFIX)/man1 for m in $(MAN); do sed "s/^\.Os sbase/.Os sbase $(VERSION)/g" < "$$m" > $(DESTDIR)$(MANPREFIX)/man1/"$$m"; done cd $(DESTDIR)$(MANPREFIX)/man1 && chmod 644 $(MAN) diff --git a/grep.c b/grep.c index ca255ff..c01d151 100644 --- a/grep.c +++ b/grep.c @@ -180,6 +180,11 @@ main(int argc, char *argv[]) SLIST_INIT(&phead); + if (!strcmp(argv[0], "egrep")) + Eflag =1, flags |= REG_EXTENDED; + else if (!strcmp(argv[0], "fgrep")) + Fflag = 1; + ARGBEGIN { case 'E': Eflag = 1; -- 2.1.4
Re: [hackers] [PATCH] yellow italics everywhere is for colorblind people
On Tue, Jan 05, 2016 at 05:44:12PM +0800, Pickfire wrote: > On Tue, Jan 05, 2016 at 10:42:52AM +0100, FRIGN wrote: > >>Hi, I use `git send-email`, it won't be mentioned by default. > >>It is for st. As you can see in the patch. > > You can use git send-email --subject-prefix='st][PATCH'
[hackers] [sbase][PATCH] ed: Use TMPDIR to locate the temporal file
The current behaviour of storing the scratch file in the current directory is a bit painful, because it generates files in all the directories where you execute ed. BSD ed uses TMPDIR for this purpouse, so if the user wants to put the scratch file in other place different of /tmp it only has to set this variable. --- ed.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/ed.c b/ed.c index 1ecd0d9..62f8477 100644 --- a/ed.c +++ b/ed.c @@ -329,15 +329,17 @@ static void setscratch() { int k; + char *dir; clearbuf(); clearundo(); - strcpy(tmpname, "ed.XX"); + if ((dir = getenv("TMPDIR")) == NULL) + dir = "/tmp/"; + if (strlen(dir) + sizeof("ed.XX") > FILENAME_MAX) + error("incorrect scratch file name"); + strcat(strcpy(tmpname, dir), "ed.X"); if ((scratch = mkstemp(tmpname)) < 0) { - /* try /tmp if cwd is not writable */ - strcpy(tmpname, "/tmp/ed.XX"); - if ((scratch = mkstemp(tmpname)) < 0) - error("failed to create scratch file"); + error("failed to create scratch file"); } if ((k = makeline("", NULL))) error("input/output error in scratch file"); -- 2.1.4
[hackers] [sbase][PATCH 1/3] ed: Correct error message when open file
"input/output" error was to general and could create confusion. All the other ed implementations give a "cannot open input file" --- ed.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ed.c b/ed.c index 7e7fbb6..96cfc3b 100644 --- a/ed.c +++ b/ed.c @@ -609,8 +609,8 @@ doread(char *fname) if (fp) fclose(fp); - if (!(fp = fopen(fname, "r"))) - error("input/output error"); + if ((fp = fopen(fname, "r")) == NULL) + error("cannot open input file"); curln = line2; for (cnt = 0; (n = getline(&s, &len, fp)) > 0; cnt += (size_t)n) { -- 2.1.4
[hackers] [sbase][PATCH 3/3] ed: Fix error introduced in b19d708
This patch introduced init() function, which removed the initialization code of savfname in doread, but this is incorrect, because savfname can be initialized with a r command if savfname is empty. --- ed.c | 19 +++ 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/ed.c b/ed.c index 5369d60..1ecd0d9 100644 --- a/ed.c +++ b/ed.c @@ -702,7 +702,7 @@ chkprint(int flag) } static char * -getfname(void) +getfname(char comm) { int c; char *bp; @@ -721,6 +721,8 @@ getfname(void) error("file name too long"); } else { *bp = '\0'; + if (savfname[0] == '\0' || comm == 'e' || comm == 'f') + strcpy(savfname, fname); return fname; } return NULL; /* not reached */ @@ -1015,7 +1017,7 @@ subst(int nth) static void docmd(void) { - char *s, cmd; + char cmd; int rep = 0, c, line3, num, trunc; repeat: @@ -1073,13 +1075,13 @@ repeat: trunc = 1; case 'W': deflines(nextln(0), lastln); - dowrite(getfname(), trunc); + dowrite(getfname(cmd), trunc); break; case 'r': if (nlines > 1) goto bad_address; deflines(lastln, lastln); - doread(getfname()); + doread(getfname(cmd)); break; case 'd': chkprint(1); @@ -1190,10 +1192,11 @@ repeat: case 'f': if (nlines > 0) goto unexpected; - if (!strcmp(s = getfname(), savfname)) - puts(savfname); + if (back(input()) != '\n') + getfname(cmd); else - strcpy(savfname, s); + puts(savfname); + chkprint(0); break; case 'E': modflag = 0; @@ -1202,7 +1205,7 @@ repeat: goto unexpected; if (modflag) goto modified; - strcpy(savfname, getfname()); + getfname(cmd); setscratch(); deflines(curln, curln); doread(savfname); -- 2.1.4
[hackers] [sbase][PATCH 2/3] ed: Don't show '!' in exec with -s
POSIX indicates that this '!' is a diagnosis that must not be printed when -s is supplied. --- ed.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ed.c b/ed.c index 96cfc3b..5369d60 100644 --- a/ed.c +++ b/ed.c @@ -871,7 +871,8 @@ execsh(void) if (repl) puts(cmd); system(cmd); - puts("!"); + if (optdiag) + puts("!"); } static void -- 2.1.4
Re: [hackers] [sbase] [PATCH] ed: Do not try to read-in a nonexistant file
On Sat, Dec 26, 2015 at 05:02:39PM -0500, Wolfgang Corcoran-Mathe wrote: > This fixes a segfault caused by running ed with a > nonexistant filename argument, e.g. 'ed not_a_file_yet'. Good catch, but I don't like the solution. I think you are fixing the problem in the incorrect place. The problem here is not that the file doesn't exists, but the problem is we are calling undo(), which tries to undo the work of the current command, which is controled by newcmd, and newcmd is 0 because we are out of edit(). It causes that doread() calls to error(), which calls to undo() which calls to error(), and then we have an infinite recursivity. I think the solution must be removing the call to error() in undo(). I think something like this patch solves the problem: diff --git a/config.mk b/config.mk index 9fb18da..3ca45d2 100644 --- a/config.mk +++ b/config.mk @@ -12,5 +12,5 @@ RANLIB = ranlib # for NetBSD add -D_NETBSD_SOURCE # -lrt might be needed on some systems CPPFLAGS = -D_DEFAULT_SOURCE -D_BSD_SOURCE -D_XOPEN_SOURCE=700 -D_FILE_OFFSET_BITS=64 -CFLAGS = -std=c99 -Wall -pedantic -LDFLAGS = -s +CFLAGS = -g -std=c99 -Wall -pedantic +LDFLAGS = -g diff --git a/ed.c b/ed.c index 8903957..623c6b4 100644 --- a/ed.c +++ b/ed.c @@ -282,7 +282,7 @@ undo(void) struct link *p; if (udata.nr == 0) - error("nothing to undo"); + return; for (p = &udata.vec[udata.nr-1]; udata.nr--; --p) { zero[p->from1].next = p->to1; zero[p->from2].prev = p->to2; @@ -1101,6 +1101,8 @@ repeat: if (nlines > 0) goto bad_address; chkprint(1); + if (udata.nr == 0) + error("nothing to undo"); undo(); break; case 's': Regards,
Re: [hackers][sbase][PATCH] Activate the "else if" branch
On Tue, Dec 15, 2015 at 07:54:28PM +0100, Silvan Jegen wrote: > We checked the same condition in the "if" branch so it was never true > in the "else if" one. Removing this condition makes the "else if" > branch viable. I'm sorry, but you are wrong here. Setjmp saves the current state of the program, and it allows that a deeper call to longjmp restores the state. When setjmp is called directly it returns always 0, but when it returns dur to a call to longjmp it returns a value passed as parameter to longjmp (in our case 1). It is a kind of try {} catch{} ala C. > dowrite("ed.hup", 1); > - } else if (home && !setjmp(savesp)) { > + } else if (home) { > n = snprintf(fname, If you remove the setjmp in the else if branch, then any call to error (which calls to longjmp) will resume the execution in the if branch, making a new execution of the else if branch, which in some cases will produce an infinite loop. Regards,
[hackers] [sbase][PATCH] Check if PRIO_MIN and PRIO_MAX are defined
The majority of the systems define PRIO_MAX and PRIO_MIN, but there is an obscure system, whose name I am not going to tell, where they were not defined. --- nice.c | 8 renice.c | 9 + 2 files changed, 17 insertions(+) diff --git a/nice.c b/nice.c index 3b9b9fb..d036e26 100644 --- a/nice.c +++ b/nice.c @@ -7,6 +7,14 @@ #include "util.h" +#ifndef PRIO_MIN +#define PRIO_MIN -NZERO +#endif + +#ifndef PRIO_MAX +#define PRIO_MAX (NZERO-1) +#endif + static void usage(void) { diff --git a/renice.c b/renice.c index 38853b8..5159558 100644 --- a/renice.c +++ b/renice.c @@ -7,6 +7,15 @@ #include "util.h" +#ifndef PRIO_MIN +#define PRIO_MIN -NZERO +#endif + +#ifndef PRIO_MAX +#define PRIO_MAX (NZERO-1) +#endif + + static int renice(int which, int who, long adj) { -- 2.1.4
Re: [hackers] [dmenu][RFC][PATCH] History functionality
On Wed, Dec 09, 2015 at 10:31:09AM +0100, Silvan Jegen wrote: > I realized that I am not dealing with the case that the history file > does not exist already. I added a simple check for that (although I > was considering just putting in a comment saying that it has to). > > +if [ ! -e $historyfile ]; then > + touch $historyfile > +fi Why the if?, why not directly touch the file? Regards,
Re: [hackers] [dmenu][RFC][PATCH 0/4] Using sort and simple C program to get dmenu history functionality
On Mon, Nov 30, 2015 at 06:51:02PM +0100, Hiltjo Posthuma wrote: > Something like (quick hack): > > cat historyfile | awk '//{x[$0]++; } END { for (k in x) { print x[k] " >" k; }}' | sort -k 1rn,2 | cut -f 2- | dmenu >> historyfile > Avoid the death cat!!!. Use something like: awk '{x[$0]++} END {for (k in x) { print x[k],k; }}' historyfile | sort -k 1rn,2 | cut -f 2- | dmenu >> historyfile Regards,
Re: [hackers] [scc] Fix character sequences || Roberto E. Vargas Caballero
On Wed, Nov 25, 2015 at 03:04:22AM -0800, Robert Ransom wrote: > On 11/24/15, g...@suckless.org wrote: > > commit 1aa2143073c30f374c33e0288135dc3e04494588 > > Author: Roberto E. Vargas Caballero > > AuthorDate: Tue Nov 24 20:29:45 2015 +0100 > > Commit: Roberto E. Vargas Caballero > > CommitDate: Tue Nov 24 20:29:45 2015 +0100 > > > + case '\'': c = '\\'; return c; > > I think this is supposed to be c = '\'' . Yes, you are right, it should be '\''. Pancake, are you going to send the patch about return c = ...? If you send it you can also fix this error. In other case, please let me know it, and I will create the patch. Regards,
Re: [hackers] [scc] Fix character sequences || Roberto E. Vargas Caballero
On Tue, Nov 24, 2015 at 09:24:59PM +0100, pancake wrote: > Why not return c = 'x'; ? > Good point!. In fact, in a previous version, it was written in this way. Can you send a patch with it? Regards,
Re: [hackers] [st] startup options
> Mainly I want scroll when compiling. It print a lot of information and > warnings. And I want to read them clearly without interrupting my > compilation. Other than that I don't use scrolling much. > > ofcourse, i can redirect std err to a log file and see it. But when I run > "make" I need to redirect it to a file and then tail that file. I would be > glad if there is some other workaround. Do you know what is a pager? Maybe you could use something more or less (pun intended)
[hackers] [st][PATCH] Do not mark as invalid UTF8 control codes
wcwidth() returns -1 in all the non visible characters, but it doesn't mind that they are incorrect. It only means that they are not printable. --- st.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/st.c b/st.c index 1df4fde..35a840b 100644 --- a/st.c +++ b/st.c @@ -2895,15 +2895,15 @@ tputc(Rune u) int width, len; Glyph *gp; + control = ISCONTROL(u); len = utf8encode(u, c); - if ((width = wcwidth(u)) == -1) { + if (!control && (width = wcwidth(u)) == -1) { memcpy(c, "\357\277\275", 4); /* UTF_INVALID */ width = 1; } if (IS_SET(MODE_PRINT)) tprinter(c, len); - control = ISCONTROL(u); /* * STR sequence must be checked before anything else -- 2.1.4
Re: [hackers] [st] Patch to workaround missing st terminfo on remote SSH.
On Thu, Aug 13, 2015 at 12:17:35PM +0100, Nick wrote: > Quoth Roberto E. Vargas Caballero: > > I think a better aproach is to define an alias like this: > > > > alias ssh=TERM='TERM=xterm ssh' > > Syntax like that is one reason that I prefer one or two line shell > scripts to aliases. Good idea, though. Obviously is a typo: alias ssh='TERM=xterm ssh'
Re: [hackers] [st] Patch to workaround missing st terminfo on remote SSH.
On Wed, Aug 12, 2015 at 07:07:15PM +0200, Christoph Lohmann wrote: > It looks like you are redefining the value of ?term? and ?xterm?256col? > or? on systems that have st.info installed. This will create incompati? > bilities for such systems when xterm is used. ?xterm? and ?st? are not > completely compatible, otherwise there would be no need for this sepa? > rate terminfo entry. I think a better aproach is to define an alias like this: alias ssh=TERM='TERM=xterm ssh' Regards,
Re: [hackers] [scc][PATCH] Add total compability to the Makefiles
On Thu, Jul 16, 2015 at 08:41:31AM +0100, Nick wrote: > The comment line for this is wrong; it's definitely not for st. Did > this come from a commit hook? If so, it needs to be fixed. Ups, you are rigth. This patch is for scc, and it is a proposal, but like almost all the patches I send are for st, my fingers were faster than my mind and I wrote st instead of scc. sorry. Regards,
[hackers] [st][PATCH] Add total compability to the Makefiles
With .POSIX target we get that the user without environment will execute c99. If the user has no c99, then he can source env.sh or execute build.sh. --- Makefile | 2 ++ build.sh | 8 +--- cc1/Makefile | 3 ++- cc2/Makefile | 2 ++ env.sh | 23 +++ lib/Makefile | 2 ++ 6 files changed, 32 insertions(+), 8 deletions(-) create mode 100755 env.sh diff --git a/Makefile b/Makefile index 76176de..cfdf573 100644 --- a/Makefile +++ b/Makefile @@ -6,3 +6,5 @@ all clean: do \ (cd $$i && $(MAKE) $@) ;\ done + +.POSIX: diff --git a/build.sh b/build.sh index e280799..dea6489 100755 --- a/build.sh +++ b/build.sh @@ -1,11 +1,5 @@ #!/bin/sh -case `uname` in -Plan9) - CFLAGS="-D_SUSV2_SOURCE -DNBOOL" - export CFLAGS - ;; -esac - +source env.sh make $@ diff --git a/cc1/Makefile b/cc1/Makefile index ed8ac07..47e9c09 100644 --- a/cc1/Makefile +++ b/cc1/Makefile @@ -1,9 +1,10 @@ -CFLAGS = -ansi OBJS = types.o decl.o lex.o error.o symbol.o main.o expr.o \ code.o stmt.o cpp.o all: cc1 +.POSIX: + $(OBJS) : cc1.h ../inc/cc.h ../inc/sizes.h cc1: $(OBJS) ../lib/libcc.a diff --git a/cc2/Makefile b/cc2/Makefile index 2e364fa..1324c82 100644 --- a/cc2/Makefile +++ b/cc2/Makefile @@ -5,6 +5,8 @@ LIBS = -lcc all: cc2 +.POSIX: + $(OBJS): ../inc/cc.h ../inc/sizes.h cc2.h main.o: error.h diff --git a/env.sh b/env.sh new file mode 100755 index 000..30f3b88 --- /dev/null +++ b/env.sh @@ -0,0 +1,23 @@ +#!/bin/sh + +case `uname` in +Plan9) + CFLAGS="-D_SUSV2_SOURCE -DNBOOL" + export CFLAGS + ;; +*) + case x$CC in + xc99) + ;; + x|xgcc) + CC=gcc + CFLAGS=-std=c99 + export CFLAGS CC + ;; + *) + echo You need a c99 compiler for this program 2>&1 + exit + ;; + esac + ;; +esac diff --git a/lib/Makefile b/lib/Makefile index 586c8b9..3e3a0c0 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -3,6 +3,8 @@ OBJS = die.o xcalloc.o xmalloc.o xrealloc.o xstrdup.o all: libcc.a +.POSIX: + libcc.a: $(OBJS) ar r $@ $? -- 2.1.4