Re: [dev] [sinit] 0.9 release
On Sat, Apr 19, 2014 at 9:13 AM, Krol, Willem van de <008...@jfc.nl> wrote: > Isn't setpgid(0, 0) after setsid() redundant? > No. [0] setsid() creates a new session. cheers! mar77i [0] http://pubs.opengroup.org/onlinepubs/009695399/functions/setsid.html
Re: [dev] [sinit] 0.9 release
On Tue, Apr 22, 2014 at 09:09:31AM +0200, Martti Kühne wrote: > On Sat, Apr 19, 2014 at 9:13 AM, Krol, Willem van de <008...@jfc.nl> wrote: > > Isn't setpgid(0, 0) after setsid() redundant? > > > > No. [0] setsid() creates a new session. The issue is related to setsid() - in particular if you use setpgid(0, 0) to move a process from one process group to another the pgid specifies an existing process group to be joined and the session ID of that group must match the session ID of the joining process. So if you try the code we had (with setstid() + setpgid(0, 0)) you will see that the setpgid() call returns -EPERM. So it is not redundant, it is plain wrong. That is if I understand everything correctly :P Cheers, sin
[dev] [st] [PATCH 3/6] Remove argument names from function prototypes.
---
st.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/st.c b/st.c
index 6da4827..64384cd 100644
--- a/st.c
+++ b/st.c
@@ -360,7 +360,7 @@ static void strparse(void);
static void strreset(void);
static int tattrset(int);
-static void tprinter(char *s, size_t len);
+static void tprinter(char *, size_t);
static void tdumpsel(void);
static void tdumpline(int);
static void tdump(void);
@@ -371,7 +371,7 @@ static void tdeleteline(int);
static void tinsertblank(int);
static void tinsertblankline(int);
static void tmoveto(int, int);
-static void tmoveato(int x, int y);
+static void tmoveato(int, int);
static void tnew(int, int);
static void tnewline(int);
static void tputtab(bool);
@@ -380,7 +380,7 @@ static void treset(void);
static int tresize(int, int);
static void tscrollup(int, int);
static void tscrolldown(int, int);
-static void tsetattr(int*, int);
+static void tsetattr(int *, int);
static void tsetchar(char *, Glyph *, int, int);
static void tsetscroll(int, int);
static void tswapscreen(void);
@@ -413,9 +413,9 @@ static void xsettitle(char *);
static void xresettitle(void);
static void xsetpointermotion(int);
static void xseturgency(int);
-static void xsetsel(char*);
+static void xsetsel(char *);
static void xtermclear(int, int, int, int);
-static void xunloadfont(Font *f);
+static void xunloadfont(Font *);
static void xunloadfonts(void);
static void xresize(int, int);
@@ -453,7 +453,7 @@ static size_t utf8validate(long *, size_t);
static ssize_t xwrite(int, char *, size_t);
static void *xmalloc(size_t);
static void *xrealloc(void *, size_t);
-static char *xstrdup(char *s);
+static char *xstrdup(char *);
static void (*handler[LASTEvent])(XEvent *) = {
[KeyPress] = kpress,
--
1.8.4
[dev] [st] [PATCH 2/6] Style fix in tdumpsel.
---
st.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/st.c b/st.c
index 92fd7da..6da4827 100644
--- a/st.c
+++ b/st.c
@@ -2256,8 +2256,7 @@ printsel(const Arg *arg) {
}
void
-tdumpsel(void)
-{
+tdumpsel(void) {
char *ptr;
if((ptr = getsel())) {
--
1.8.4
[dev] [st] [PATCH 6/6] Make xrealloc and xstrdup style consistent.
---
st.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/st.c b/st.c
index bb3e687..aba034f 100644
--- a/st.c
+++ b/st.c
@@ -549,12 +549,10 @@ xrealloc(void *p, size_t len) {
char *
xstrdup(char *s) {
- char *p = strdup(s);
-
- if (!p)
+ if((s = strdup(s)) == NULL)
die("Out of memory\n");
- return p;
+ return s;
}
size_t
--
1.8.4
[dev] [st] [PATCH 4/6] Use uint and uchar instead of unsigned int and unsigned char.
---
st.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/st.c b/st.c
index 64384cd..b8bf84b 100644
--- a/st.c
+++ b/st.c
@@ -298,13 +298,13 @@ typedef struct {
typedef union {
int i;
- unsigned int ui;
+ uint ui;
float f;
const void *v;
} Arg;
typedef struct {
- unsigned int mod;
+ uint mod;
KeySym keysym;
void (*func)(const Arg *);
const Arg arg;
@@ -3076,7 +3076,7 @@ xinit(void) {
xw.netwmpid = XInternAtom(xw.dpy, "_NET_WM_PID", False);
XChangeProperty(xw.dpy, xw.win, xw.netwmpid, XA_CARDINAL, 32,
- PropModeReplace, (unsigned char *)&thispid, 1);
+ PropModeReplace, (uchar *)&thispid, 1);
xresettitle();
XMapWindow(xw.dpy, xw.win);
--
1.8.4
[dev] [st] [PATCH 5/6] Use BETWEEN in tsetchar.
---
st.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/st.c b/st.c
index b8bf84b..bb3e687 100644
--- a/st.c
+++ b/st.c
@@ -1544,8 +1544,7 @@ tsetchar(char *c, Glyph *attr, int x, int y) {
* The table is proudly stolen from rxvt.
*/
if(attr->mode & ATTR_GFX) {
- if(c[0] >= 0x41 && c[0] <= 0x7e
- && vt100_0[c[0] - 0x41]) {
+ if(BETWEEN(c[0], 0x41, 0x7e) && vt100_0[c[0] - 0x41]) {
c = vt100_0[c[0] - 0x41];
}
}
--
1.8.4
[dev] [st] [PATCH 1/6] Use BETWEEN in tinsertblankline and tdeleteline.
---
st.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/st.c b/st.c
index 019f53c..92fd7da 100644
--- a/st.c
+++ b/st.c
@@ -1628,18 +1628,14 @@ tinsertblank(int n) {
void
tinsertblankline(int n) {
- if(term.c.y < term.top || term.c.y > term.bot)
- return;
-
- tscrolldown(term.c.y, n);
+ if(BETWEEN(term.c.y, term.top, term.bot))
+ tscrolldown(term.c.y, n);
}
void
tdeleteline(int n) {
- if(term.c.y < term.top || term.c.y > term.bot)
- return;
-
- tscrollup(term.c.y, n);
+ if(BETWEEN(term.c.y, term.top, term.bot))
+ tscrollup(term.c.y, n);
}
int32_t
--
1.8.4
[dev] [st] [PATCH] Fix techo handling of control characters.
Internally st represents characters using "char" type.
It is used in CSIEscape.buf, Glyph.c etc.
However, char can be either signed or unsigned depends on the
architecture.
On x86 '\x80' < 0x20 is true, but (uchar)'\x80' < 0x20 is false.
tputc explicitly converts character to ascii code:
uchar ascii = *c;
In tsetchar there is this code:
c[0] >= 0x41 && c[0] <= 0x7e
This condition is false for negative chars, so, accidentally, it works
the same way for signed and unsigned chars.
However, techo compares signed char to '\x20' and has a bug.
How to reproduce:
1. Add the following keybinding:
{ XK_F1,XK_NO_MOD, "\x80" , 0,0,0},
2. Run st and enable echo mode: printf '\e[12l'
3. Press F1. Character '\x80' is recognized as control and ^ is displayed,
followed by unprintable character.
This patch fixes the bug the same way it is fixed in tputc.
Also techo did not recognize DEL as control character and did not
display ^? for it, this patch fixes this bug too.
---
st.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/st.c b/st.c
index 019f53c..3bf8eee 100644
--- a/st.c
+++ b/st.c
@@ -2315,10 +2315,12 @@ void
techo(char *buf, int len) {
for(; len > 0; buf++, len--) {
char c = *buf;
+ uchar ascii = c;
+ bool control = ascii < '\x20' || ascii == 0177;
- if(c < '\x20') { /* control code */
+ if(control) { /* control code */
if(c != '\n' && c != '\r' && c != '\t') {
- c |= '\x40';
+ c ^= '\x40';
tputc("^", 1);
}
tputc(&c, 1);
--
1.8.4
Re: [dev] [st] [PATCH] Fix techo handling of control characters.
BTW what do you think about converting st code to store characters in uchar instead of char to avoid these problems in the future and avoid manual conversions? tsetchar should probably be fixed too. c[0] >= 0x41 && c[0] <= 0x7e works, but it is better to change type of `c' to uchar *.
Re: [dev] [st] [patch] misplaced parenthesis in LEN macro
On Sun, Apr 20, 2014 at 9:24 PM, Rob wrote: > Into the bikeshed I go... > > LEN(a + 2) doesn't mean anything anyway, as a's type decays. > > To do it properly there should be some kind of static assert in the > macro that the argument is of array type. But this is a small code base > and you'd expect that the code would be run and checked before > committing, which renders the assert pretty useless. > > I think it's fine as it is, in the original C way of doing things, > garbage in, garbage out, undefined behaviour etc etc. > > Rob > I may remind you there is the case where people make struct concatenations, just because they can. Arrays of concatenated structs. The cases where you don't even care when the preprocessor will append a pointer or a size_t to your type. You don't even want to know. So, no, the parentheses are not just needed for style. Which we require therefore. Thanks for your time. cheers! mar77i
Re: [dev] [PATCH] [st 1/3] Use tsetdirt in tscrollup and tscrolldown.
Hi, I like these patches, and I want to apply them, but there is a small thing that I think should be modified. After the patches tscolldown has: tsetdirt(orig, term.bot-n); tclearregion(0, term.bot-n+1, term.col-1, term.bot); but tscrollup has: tclearregion(0, orig, term.col-1, orig+n-1); tsetdirt(orig+n, term.bot); Is there any reason to do it in different order?. I think is more understable if the order is the same in both cases. Regards, -- Roberto E. Vargas Caballero
Re: [dev] [st] [PATCH 1/6] Use BETWEEN in tinsertblankline and tdeleteline.
Hi, I am going to apply the full set of patches. Thanks!! -- Roberto E. Vargas Caballero
Re: [dev] [PATCH] [st 1/3] Use tsetdirt in tscrollup and tscrolldown.
On Tue, Apr 22, 2014 at 09:34:58PM +0200, Roberto E. Vargas Caballero wrote: > Hi, > > I like these patches, and I want to apply them, but there is a > small thing that I think should be modified. After the patches tscolldown > has: > > tsetdirt(orig, term.bot-n); > tclearregion(0, term.bot-n+1, term.col-1, term.bot); > > but tscrollup has: > > tclearregion(0, orig, term.col-1, orig+n-1); > tsetdirt(orig+n, term.bot); > > Is there any reason to do it in different order?. I think is more > understable if the order is the same in both cases. They are sorted by row number. In tscrolldown we set dirty orig..term.bot-n and then term.bot-n+1..term.bot In tscrollup we set dirty orig..orig+n-1 and then orig+n..term.bot If you concatenate these ranges, you get orig..term.bot.
Re: [dev] [st] [PATCH] Fix techo handling of control characters.
> It is used in CSIEscape.buf, Glyph.c etc.
> However, char can be either signed or unsigned depends on the
> architecture.
It depends of the compiler, not of the architecture. Compiler will decide
signess of char based in what can be more efficient or what is
more logical.
> On x86 '\x80' < 0x20 is true, but (uchar)'\x80' < 0x20 is false.
Here you should say that gcc (I suppose you are using it) in x86
evaluate these expressions in this way, but be careful because maybe
another compiler in x86 can do another thing.
> tputc explicitly converts character to ascii code:
> uchar ascii = *c;
It converts it to unsigned char, the character is not translated
from a code to another. Ascii is only a name of a variable.
>
> In tsetchar there is this code:
> c[0] >= 0x41 && c[0] <= 0x7e
> This condition is false for negative chars, so, accidentally, it works
> the same way for signed and unsigned chars.
It is not an accident. C standard ensures that any printable
character has the most significant bit to 0, and this is basically why the
code works, because doesn't matter if char is signed or unsigned. In other
case all the functions of ctype could be not implementable.
> How to reproduce:
> 1. Add the following keybinding:
> { XK_F1,XK_NO_MOD, "\x80" , 0,0,0},
> 2. Run st and enable echo mode: printf '\e[12l'
> 3. Press F1. Character '\x80' is recognized as control and ^ is displayed,
> followed by unprintable character.
This is another different problem. St is an utf8 terminal, so
any character with the most significant bit to 1 is part of an
utf8 sequence, so the key assignation you did was incorrect for
an utf8 terminal (it put utf8decoder in an incorrect state).
> Also techo did not recognize DEL as control character and did not
> display ^? for it, this patch fixes this bug too.
This is the part of the patch I like, because it is true that DEL was not
not correctly handled (it is the reason why tputc has this check).
If you modify the commit message and explain the problem with the DEL
as the reason for the commit I will apply it.
Regards,
--
Roberto E. Vargas Caballero
[dev] [st] [PATCH] Fix techo handling of control and multibyte characters.
techo compares signed char to '\x20'. Any character with code less then
'\x20' is treated as control character. This way characters with MSB
set to 1 are considered control characters too.
Also this patch makes techo display DEL character as ^?.
To reprocuce the bug, enable echo mode using printf '\e[12l',
then type DEL character or any non-ASCII character.
---
st.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/st.c b/st.c
index 019f53c..b6cb01c 100644
--- a/st.c
+++ b/st.c
@@ -2316,9 +2316,9 @@ techo(char *buf, int len) {
for(; len > 0; buf++, len--) {
char c = *buf;
- if(c < '\x20') { /* control code */
+ if(c < 0x20u || c == 0177) { /* control code */
if(c != '\n' && c != '\r' && c != '\t') {
- c |= '\x40';
+ c ^= '\x40';
tputc("^", 1);
}
tputc(&c, 1);
--
1.8.4
Re: [dev] lock (1) - where does it go?
> How are you getting on with this? I am planning to do a bit more polishing > to ubase + add 1-2 tools and then make an initial v0.1 release. Would wouldn't this fit better in sbase?
Re: [dev] lock (1) - where does it go?
On Tue, Apr 22, 2014 at 05:20:29PM -0400, Calvin Morrison wrote: > > How are you getting on with this? I am planning to do a bit more polishing > > to ubase + add 1-2 tools and then make an initial v0.1 release. Would > > wouldn't this fit better in sbase? depends... a naive implementation can be done portably and fit in sbase. if we put it in ubase, I suspect we'll fix it to work with inotify instead.
Re: [dev] lock (1) - where does it go?
On 22 April 2014 17:24, sin wrote: > On Tue, Apr 22, 2014 at 05:20:29PM -0400, Calvin Morrison wrote: >> > How are you getting on with this? I am planning to do a bit more polishing >> > to ubase + add 1-2 tools and then make an initial v0.1 release. Would >> >> wouldn't this fit better in sbase? > > depends... a naive implementation can be done portably and fit in sbase. > if we put it in ubase, I suspect we'll fix it to work with inotify instead. > might as well use inotify tools then inotifywait -e delete lock_dir
Re: [dev] lock (1) - where does it go?
On Tue, Apr 22, 2014 at 05:34:47PM -0400, Calvin Morrison wrote: > On 22 April 2014 17:24, sin wrote: > > On Tue, Apr 22, 2014 at 05:20:29PM -0400, Calvin Morrison wrote: > >> > How are you getting on with this? I am planning to do a bit more > >> > polishing > >> > to ubase + add 1-2 tools and then make an initial v0.1 release. Would > >> > >> wouldn't this fit better in sbase? > > > > depends... a naive implementation can be done portably and fit in sbase. > > if we put it in ubase, I suspect we'll fix it to work with inotify instead. > > > > might as well use inotify tools then > > inotifywait -e delete lock_dir the point is to have something light and self-contained. you might as well use gnu coreutils then.
[dev] [PATCH] Simplify tdeletechar and tinsertblank and fix memory corruption.
Current CSI parsing code uses strtol to parse arguments and allows them
to be negative. Negative argument is not properly handled in tdeletechar
and tinsertblank and results in memory corruption in memmove.
Reproduce with printf '\e[-500@'
Patch also removes special handling for corner case and simplifies
the code.
Removed
term.dirty[term.c.y] = 1
because tclearregion sets dirty flag.
---
st.c | 26 ++
1 file changed, 10 insertions(+), 16 deletions(-)
diff --git a/st.c b/st.c
index 019f53c..ae76731 100644
--- a/st.c
+++ b/st.c
@@ -1592,16 +1592,13 @@ tclearregion(int x1, int y1, int x2, int y2) {
void
tdeletechar(int n) {
- int src = term.c.x + n;
- int dst = term.c.x;
- int size = term.col - src;
+ int dst, src, size;
- term.dirty[term.c.y] = 1;
+ LIMIT(n, 0, term.col - term.c.x);
- if(src >= term.col) {
- tclearregion(term.c.x, term.c.y, term.col-1, term.c.y);
- return;
- }
+ dst = term.c.x;
+ src = term.c.x + n;
+ size = term.col - src;
memmove(&term.line[term.c.y][dst], &term.line[term.c.y][src],
size * sizeof(Glyph));
@@ -1610,16 +1607,13 @@ tdeletechar(int n) {
void
tinsertblank(int n) {
- int src = term.c.x;
- int dst = src + n;
- int size = term.col - dst;
+ int dst, src, size;
- term.dirty[term.c.y] = 1;
+ LIMIT(n, 0, term.col - term.c.x);
- if(dst >= term.col) {
- tclearregion(term.c.x, term.c.y, term.col-1, term.c.y);
- return;
- }
+ dst = term.c.x + n;
+ src = term.c.x;
+ size = term.col - dst;
memmove(&term.line[term.c.y][dst], &term.line[term.c.y][src],
size * sizeof(Glyph));
--
1.8.4
[dev][sandy][patch] minor typo correction
Minor typo correction. diff --git a/TODO b/TODO index 6e01237..535a8e4 100644 --- a/TODO +++ b/TODO @@ -13,7 +13,7 @@ In no particular order, at sandy.c: At config.def.h: - Bindings! - Use local/system script path and all -- Choose color-set for either black of white bg +- Choose color-set for either black or white bg - Define more syntaxes - Refine syntax regexes typo.txt Description: Binary data
