Re: [hackers] [sbase] [PATCH] find: Fix unterminated array
On Sat, Jun 13, 2015 at 10:42 PM, Wolfgang Corcoran-Mathe first.lord.of.t...@gmail.com wrote: This caused a segfault with semicolon-terminated -exec primaries. good catch, needs to be done in get_ok_arg() as well, not currently somewhere I can do that, mind adding another patch? -emg
Re: [hackers] [sbase] [PATCH] find: Fix unterminated array
On Mon, Jun 15, 2015 at 9:28 AM, Evan Gates evan.ga...@gmail.com wrote: On Sat, Jun 13, 2015 at 10:42 PM, Wolfgang Corcoran-Mathe first.lord.of.t...@gmail.com wrote: This caused a segfault with semicolon-terminated -exec primaries. good catch, needs to be done in get_ok_arg() as well, not currently somewhere I can do that, mind adding another patch? -emg I'd recommend checking the isplus branch as well. If I screwed up the semicolon case chances are I screwed up the plus case, too. -emg
Re: [hackers] [sbase] [PATCH] find
emg, I noticed a few other things in get_ok_arg(), primarily that the fgets() flushing loop was causing find to hang mysteriously. Patch 2/3 should fix this, hopefully. The isplus branch seems to work, based on limited testing. I've enjoyed reading your code, btw. -- WCM
[hackers] [sbase] [PATCH 2/3] find: Fix flushing input buffer with -ok
The original flush-stdin loop (with fgets()) hung until the user
entered some extraneous characters for it to kill.
emg's FIXME about nulls still applies.
---
find.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/find.c b/find.c
index a870a90..3871105 100644
--- a/find.c
+++ b/find.c
@@ -408,16 +408,16 @@ pri_ok(struct arg *arg)
{
int status, reply;
pid_t pid;
- char ***brace, buf[256];
+ char ***brace, c;
struct okarg *o = arg-extra.p;
fprintf(stderr, %s: %s ?, *o-argv, arg-path);
reply = fgetc(stdin);
/* throw away rest of line */
- while (fgets(buf, sizeof(buf), stdin) *buf buf[strlen(buf) - 1]
== '\n')
+ while ((c = fgetc(stdin)) != '\n' c != EOF)
/* FIXME: what if the first character of the rest of the line
is a null
-* byte? probably shouldn't juse fgets() */
+* byte? */
;
if (feof(stdin)) /* FIXME: ferror()? */
--
2.3.5
[hackers] [sbase] [PATCH 1/3] find: Fix unterminated array in -ok primary
---
find.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/find.c b/find.c
index dedf5a1..a870a90 100644
--- a/find.c
+++ b/find.c
@@ -429,6 +429,7 @@ pri_ok(struct arg *arg)
/* insert filename everywhere user gave us {} */
for (brace = o-braces; *brace; brace++)
**brace = arg-path;
+ *brace = NULL;
switch((pid = fork())) {
case -1:
--
2.3.5
Re: [hackers] [sbase] [PATCH 1/3] find: Fix unterminated array in -ok primary
On Mon, Jun 15, 2015 at 12:27 PM, Wolfgang Corcoran-Mathe
first.lord.of.t...@gmail.com wrote:
---
find.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/find.c b/find.c
index dedf5a1..a870a90 100644
--- a/find.c
+++ b/find.c
@@ -429,6 +429,7 @@ pri_ok(struct arg *arg)
/* insert filename everywhere user gave us {} */
for (brace = o-braces; *brace; brace++)
**brace = arg-path;
+ *brace = NULL;
switch((pid = fork())) {
case -1:
--
2.3.5
Not needed there, *brace will already be NULL or the loop wouldn't
have stopped (the cause of the segfault). Needed in get_ok_arg() so
that it is NULL when we get here. See attached.
-emg
From d842da4c8f9fffd304858a0ae62f1c2933c97cef Mon Sep 17 00:00:00 2001
From: Evan Gates evan.ga...@gmail.com
Date: Mon, 15 Jun 2015 13:49:38 -0700
Subject: [PATCH] NULL terminate braces array in get_ok_arg
---
find.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/find.c b/find.c
index dcefca5..e9d8c1d 100644
--- a/find.c
+++ b/find.c
@@ -635,6 +635,7 @@ get_ok_arg(char *argv[], union extra *extra)
for (arg = argv, braces = o-braces; *arg; arg++)
if (!strcmp(*arg, {}))
*braces++ = arg;
+ *braces = NULL;
gflags.print = 0;
return arg;
--
2.4.3
Re: [hackers] [sbase] [PATCH 2/3] find: Fix flushing input buffer with -ok
On Mon, Jun 15, 2015 at 12:27 PM, Wolfgang Corcoran-Mathe first.lord.of.t...@gmail.com wrote: emg's FIXME about nulls still applies. /* throw away rest of line */ - while (fgets(buf, sizeof(buf), stdin) *buf buf[strlen(buf) - 1] == '\n') + while ((c = fgetc(stdin)) != '\n' c != EOF) /* FIXME: what if the first character of the rest of the line is a null -* byte? probably shouldn't juse fgets() */ +* byte? */ ; Nope, not a problem anymore as you're explicitly checking against '\n' and EOF so reading a '\0' is fine. Much better than the fgets. -emg
Re: [hackers] [sbase] [PATCH] find
On Mon, Jun 15, 2015 at 12:27 PM, Wolfgang Corcoran-Mathe
first.lord.of.t...@gmail.com wrote:
emg,
I noticed a few other things in get_ok_arg(), primarily that the fgets()
flushing loop was causing find to hang mysteriously. Patch 2/3 should
fix this, hopefully.
Should have said buf[strlen(buf) - 1] != '\n' in order to keep
reading until we got a full newline terminated line of input, not sure
how that got through. I prefer your solution as it doesn't run into
any of the '\0' problems.
The isplus branch seems to work, based on limited testing.
Yep, it looks alright, the bug was specific to the array of pointers
to {} used in -ok and -exec \;
I had originally thought it was a bug relating to NULL termination for
execvp() when reading your mail (I finally have the code in front of
me, that's helpful).
I've enjoyed reading your code, btw.
Thanks! Good to hear. Criticism, constructive or otherwise, is always welcome.
-emg
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] [sbase] [PATCH 1/3] find: Fix unterminated array in -ok primary
Quoth Evan Gates on Mon, Jun 15 2015 13:51 -0700:
On Mon, Jun 15, 2015 at 12:27 PM, Wolfgang Corcoran-Mathe
first.lord.of.t...@gmail.com wrote:
---
find.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/find.c b/find.c
index dedf5a1..a870a90 100644
--- a/find.c
+++ b/find.c
@@ -429,6 +429,7 @@ pri_ok(struct arg *arg)
/* insert filename everywhere user gave us {} */
for (brace = o-braces; *brace; brace++)
**brace = arg-path;
+ *brace = NULL;
switch((pid = fork())) {
case -1:
--
2.3.5
Not needed there, *brace will already be NULL or the loop wouldn't
have stopped (the cause of the segfault). Needed in get_ok_arg()
so that it is NULL when we get here. See attached.
It occurred to me shortly after sending that patch that this should
have been in get_ok_arg(), as with get_exec_arg(). But without this
patch (or yours), I get a segfault when the end of the arg list is
reached. Your patch is obviously the right one, but I would like
to understand why both seem to have the same effect.
There is another semi-bug in -ok's input parsing. If the char read
into reply in pri_ok() at line 415 is a newline the user will have
to enter another newline to skip the file. This seems clumsy, but
it might be what you want.
Regards,
--
WCM
