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, );
- } else {
+ break;
+ case '\\':
+ c = input();
+ if (c != '%') {
+ back(c);
+ c = '\\';
+ }
+ default:
addchar(c, );
}
}
--
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, );
+ 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, );
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(, s);
+ inputidx = 0;
+}
+
+static void
+getinput(void)
+{
+ int ch;
+
+ string(, NULL);
+
+ while ((ch = getchar()) != '\n' && ch != EOF) {
+ if (ch == '\\') {
+ if ((ch = getchar()) == EOF)
+ break;
+ if (ch != '\n') {
+ ungetc(ch, stdin);
+ ch = '\\';
+ }
+ }
+ addchar(ch, );
+ }
+
+ addchar('\0', );
+ 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 < [FILENAME_MAX]; *bp++ = c) {
- if ((c = input()) == EOF || c == '\n')
+ if ((c = input()) == '\0')
break;
}
if (bp == fname) {
@@ -1034,7 +1046,7 @@ execsh(void)
error("no
[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(, , stdin) > 0) {
- if (*s == '.' && s[1] == '\n')
+ while (moreinput()) {
+ string(, NULL);
+ while ((ch = input()) != '\n' && ch != '\0')
+ addchar(ch, );
+ addchar('\n', );
+ addchar('\0', );
+
+ 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();
+ string(, 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();
+ string(, NULL);
} else if (cmd.siz) {
--cmd.siz;
repl = 1;
@@ -1048,7 +1060,7 @@ getrhs(int delim)
int c;
static String s;
- string();
+ string(, NULL);
while ((c = input()) != '\n' && c != EOF && c != delim)
addchar(c, );
addchar('\0', );
@@ -1152,7 +1164,7 @@ subline(int num, int nth)
int i, m, changed;
static String s;
- string();
+ string(, 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();
+ string(, 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([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, , , )) {
> - fprintf(stderr, "erresc: failed to fetch osc4 color %d\n", num);
> + if (xgetcolor(is_osc4 ? num : index, , , )) {
> + 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)
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(, , );
changed |= addsub(, , , nth, ++i);
+ if (eol || bol)
+ break;
}
if (!changed)
return;
--
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(, , );
- if (--nth > 0)
- continue;
- changed = 1;
- addsub(, , );
- if (nth == 0)
- break;
+ changed |= addsub(, , , nth, ++i);
}
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(, , );
changed |= addsub(, , , nth, ++i);
}
--
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, , );
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 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, , );
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
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] 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();
+ 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
[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(, , fp)) > 0; cnt += (size_t)n) {
--
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
[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
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.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,
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] [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] [patch][scc] fix parsing end of comment
Hi,
On Thu, Jul 16, 2015 at 12:19:45AM +0100, Dimitris Papastamos wrote:
diff --git a/cc1/lex.c b/cc1/lex.c
index c35e401..111c6f8 100644
--- a/cc1/lex.c
+++ b/cc1/lex.c
@@ -184,8 +184,8 @@ comment(char type)
{
if (type == '*') {
while (!eof) {
- while (readchar() != '*' !eof)
- /* nothing */
wow, I cannot believe this was written in this way. I suppose
there are millions of errors in the code now, because I was in an
expansion phase.
I usually work in this projects in two phases, one phase of
adding functionality, without checking too much, and then another
phase of checking and rewriting. If I try to keep it working
perfectly and well written, it is impossible to advance.
I would have put the semicolon on the same line as the while.
This is something of style. I took this style from the source
code of git, and I think it is also used in the kernel, isn't it?.
I don't have problems if it is changed.
Regards
[hackers] St style changes
Hi, We are doing deep changes of style in st, and it means the style will not be ready until 2 or three weeks, so if you have to update some of your patches in the wiki, then it is better wait a bit (mainly because in other case you will have to update your patch several times). Thank you,
Re: [hackers] [st] [patch] use goto in xloadfonts
Hi, On Sat, Jun 20, 2015 at 01:32:45AM -0400, Michael Reed wrote: You just made the programmflow harder to grasp and removed the possibility to differentiate between the errors in the future. Also the patch adds 4 SLoC without achieving anything. I agree it's harder to grasp, but only slightly, and I think the decrease in redundancy is worth it (although that's evidently subjective). I personally don't think is harder. I think is a common pattern in C to have die calls in the end of the function and gotos to them. The main problem that I can see here is the label, whose name is not significative enough: err, which error?, and if we have to add more errors in the future, do we have to change this label and all the gotos to this label?. I think a label like 'cant_open_font' is better. I usually like short names, but in the case of goto the story is totally different, because you have a modification in the control flow and it should be clear why it is done. I don't agree with the comment on error handling; it's not clear to me when/if this function will be refactored, but I don't know if gotos have complicated refactoring st in the past, so maybe you're right. The problem here is different. St was broken in the past due to style changes, and the history of the project is hard to read due to this kind of patches, so several suckless developers agreed in only accept patches which fix some error or add some feature. Style changes will be accepted only if there is something else in the patch. I like your patch (it makes st more similar to my style ;) ), but due to this reason I will not apply it. Regards,
Re: [hackers] [sbase][patch] find: empty line means no for -ok
Hi, Yes I most certainly did, this is what I get for submitting patches without testing. The shame. New patch attached, also protects against the glibc bug causing fgetc to hang after EOF was received. And what about if we send a patch to glibc instead? Regards,
Re: [hackers] st and combining characters
Hi,
I like the idea, but I think the patch needs some evolution.
A patch of 500 lines is usually hard of reading, and in in this case
the change is not trivial.
On Wed, Jun 10, 2015 at 11:39:25PM +0200, Joakim Sindholt wrote:
glyph now holds a union of two combining characters and a pointer to a zero
terminated array. This is purely to make the most of the space. No length is
kept and every character added past 2 causes a realloc.
And why 2 characters?. I think it is more logical to have only one, which is
the most common case, isn't it?. Maybe I'm missing something.
There's currently a problem with rendering where Xft doesn't draw the
combining characters as a part of the character they combine to, which causes
things like Hangul Jamo to be completely unusable stacks of characters instead
of the proper combined form.
Uhmm, I don't understand this point, can you explain it a bit better?,
or better, put an example.
@@ -98,6 +98,7 @@ enum glyph_attribute {
ATTR_WRAP = 1 8,
ATTR_WIDE = 1 9,
ATTR_WDUMMY = 1 10,
+ ATTR_DECOMPPTR = 1 11,
ATTR_BOLD_FAINT = ATTR_BOLD | ATTR_FAINT,
};
@@ -190,6 +191,10 @@ typedef XftColor Color;
typedef struct {
Rune u; /* character code */
+ union {
+ Rune c[2];
+ Rune *pc;
+ } dc;
We already have the field u, so why is not the union made with u and pc?
Line *alt;/* alternate screen */
bool *dirty; /* dirtyness of lines */
XftGlyphFontSpec *specbuf; /* font spec buffer used for rendering */
+ int specbuflen;
@@ -419,8 +426,9 @@ static void ttywrite(const char *, size_t);
static void tstrsequence(uchar);
static inline ushort sixd_to_16bit(int);
-static int xmakeglyphfontspecs(XftGlyphFontSpec *, const Glyph *, int, int,
int);
-static void xdrawglyphfontspecs(const XftGlyphFontSpec *, Glyph, int, int,
int);
+static int xmakeglyphfontspecs(XftGlyphFontSpec *, int, const Glyph *, int,
int, int);
+static int xmakeglyphfontspecsintermbuf(const Glyph *, int, int, int);
+static void xdrawglyphfontspecs(const XftGlyphFontSpec *, Glyph, int, int,
int, int);
Uhmmm, these names begin to seem like java names. We must find a better way to
name them,
because with names so long the only lower case style doesn't work.
/* append every set selected glyph to the selection */
@@ -984,6 +993,16 @@ getsel(void) {
continue;
ptr += utf8encode(gp-u, ptr);
+ if(gp-mode ATTR_DECOMPPTR) {
+ pc = gp-dc.pc;
+ for(pc = gp-dc.pc; *pc; pc++)
+ ptr += utf8encode(*pc, ptr);
+ } else {
+ if(gp-dc.c[0])
+ ptr += utf8encode(gp-dc.c[0], ptr);
+ if(gp-dc.c[1])
+ ptr += utf8encode(gp-dc.c[1], ptr);
+ }
}
If we define the union with u and pc we can remove the else in this code, no?
+ term.dirty[y] = 1;
+ if(term.line[y][x].mode ATTR_DECOMPPTR) {
+ p = term.line[y][x].dc.pc;
+ while(*p)
+ p++;
+ sz = (p - term.line[y][x].dc.pc) + 2;
+ term.line[y][x].dc.pc = xrealloc(term.line[y][x].dc.pc, sz *
sizeof(Rune));
+ term.line[y][x].dc.pc[sz-2] = u;
+ term.line[y][x].dc.pc[sz-1] = 0;
+ } else {
+ if(!term.line[y][x].dc.c[0]) {
+ term.line[y][x].dc.c[0] = u;
+ } else if(!term.line[y][x].dc.c[1]) {
+ term.line[y][x].dc.c[1] = u;
+ } else {
+ p = xmalloc(4 * sizeof(Rune));
+ p[0] = term.line[y][x].dc.c[0];
+ p[1] = term.line[y][x].dc.c[1];
+ p[2] = u;
+ p[3] = 0;
+ term.line[y][x].dc.pc = p;
+ term.line[y][x].mode |= ATTR_DECOMPPTR;
+ }
+ }
+}
The same. If the union is made with u almost all off this code disappear.
@@ -1711,7 +1786,7 @@ tclearregion(int x1, int y1, int x2, int y2) {
void
tdeletechar(int n) {
- int dst, src, size;
+ int dst, src, size, i;
Glyph *line;
LIMIT(n, 0, term.col - term.c.x);
@@ -1721,13 +1796,17 @@ tdeletechar(int n) {
size = term.col - src;
line = term.line[term.c.y];
+ for(i = dst; i src; i++) {
+ if(line[i].mode ATTR_DECOMPPTR)
+ line[i].dc.pc = NULL;
+ }
...
@@ -1737,6 +1816,10 @@ tinsertblank(int n) {
size = term.col - dst;
line = term.line[term.c.y];
+ for(i = src; i dst; i++) {
+ if(line[i].mode ATTR_DECOMPPTR)
+
Re: [hackers] [smdev] config.mk: default CC = cc || Hiltjo Posthuma
-#CC = musl-gcc +CC = cc cc is the default value of CC, so you don't get anything new with this patch, and you create some problems with: CC=tcc make (of course you can use make -e, but I don't see the point) Regards,
Re: [hackers] [st] Remove strsep() call || Roberto E. Vargas Caballero
first your loop is wrong. See what happens with the string ;, how many fields do you have there? 2, your code will return only 1. Second, to use directly the pointer or a variable is only a style question, and don't modify the simplicity of the loop. Regards,
Re: [hackers] [sbase] Rewrite foldline() in fold(1) || FRIGN
Hi,
+ for (p = str, col = 0; *p *p != '\n'; p++) {
+ if (!UTF8_POINT(*p) !bflag)
+ continue;
+ if (col = width) {
+ off = (sflag spacesect) ? spacesect - str : p - str;
+ if (fwrite(str, 1, off, stdout) != off)
+ eprintf(fwrite stdout:);
+ putchar('\n');
...
+ fputs(str, stdout);
It's a bit strange this fwrite, why don't you use putchar there?, and
why you check the return value of fwrite, but not the return value of
putcharor fputs?. I usually don't check the return value of write
functions and at the end of the loop I do a call to ferror.
Regards,
Re: [hackers] [sbase] Audit printenv(1) || FRIGN
On Sat, 28 Feb 2015 13:25:09 -0800 Evan Gates evan.ga...@gmail.com wrote: The arg loops can simply be for (; *argv; argv++) as the standard guarantees argv[argc] is NULL. Hey Evan, I discussed this with stateless and we came to the conclusion that the argc-approach is more idiomatic. In this case I agree with Evan, and I see the argv loop idiomatic. I have seen in several books and in diferent sources. I usually do the argv loop: for (++argv; *argv; ++argc) or while (*++argv) and if I need do a test argc something then I update the value of argc in the body. If I only need argc 0 I use *argv != 0. Regards,
Re: [hackers] [sbase] tput should be in ubase || sin
tabs -tput The following programs have been imported from OpenBSD and need replacing or cleaning up: The same applies to tabs. It needs terminfo. Regards,
Re: [hackers] [st] Add missed names of charset sequences || Roberto E. Vargas Caballero
It is a known issue with the hook, it only really monitors
the master branch.
Long time ago I did something similar for a project where I
was working. I think solution could be to add an update hook
similar to this:
#!/bin/sh
repo=`basename $PWD`
refname=$1
oldrev=$2
newrev=$3
tohackers()
{
for i in $*
do
subject=`git log -1 --prety-format:%s || %an $i`
git log -1 -p |
mail -s [$repo] $subject hackers@suckless.org
done
}
case $type,$oldrev in
commit,0+$)
tohackers `git rev-list $newrev`
;;
commit)
tohackers `git rev-list $oldrev..$newrev`
;;
.)
;;
esac
And other point, how is it possible that ed, the standard editor!!!,
is not installed in a suckless machine?!?!?!!!
Regards,
--
Roberto E. Vargas Caballero
Re: [hackers] [st] Add missed names of charset sequences || Roberto E. Vargas Caballero
This is the first patch serie in order to change the internal codification. I have pushed them in a new branch called wchar. I don't know why the hook has sent a different commit here. The head of wchar branch is e8f1308. Regards, -- Roberto E. Vargas Caballero
Re: [hackers] [ubase] Add lastlog(8) || Roberto E. Vargas Caballero
I wrote it for personal use, but now that it is going to be part
of ubase maybe is better to use die in some parts of the code, no?
+ if (ferror(last)) {
+ perror(error reading lastlog);
+ exit(EXIT_FAILURE);
+ }
...
+ if ((last = fopen(_PATH_LASTLOG, r)) == NULL) {
+ perror(_PATH_LASTLOG);
+ exit(EXIT_FAILURE);
+ }
...
+ perror(PASSWD);
+ exit(EXIT_FAILURE);
+ }
+ if ((p = strchr(line, ':')) == NULL) {
+ fputs(incorrect password file, stderr);
+ exit(-1);
+ }
+ *p = '
...
--
Roberto E. Vargas Caballero
