Re: CVS commit: src/lib/libedit (strncpy->strlcpy)
> This feels not good. > strncpy->strlcpy has repercussions like how strlcpy doesn't zero out the > remaining length and thus leaks uninitialized data. > > There has to be a reasonable way to handle these warnings instead of > rototilling which str*cpy function is used. Please read the code before commenting. Yes, I know that they are not equivalent, but in this case the destination strings are all local variables on the stack used internally only in the functions declared, to be compared or printed with other NUL-terminated strings. It is pointless to zero out the rest of the data. christos signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/lib/libedit (strncpy->strlcpy)
On Sun, May 31, 2020 at 07:24:24PM -0400, Christos Zoulas wrote: > Module Name: src > Committed By: christos > Date: Sun May 31 23:24:24 UTC 2020 > > Modified Files: > src/lib/libedit: terminal.c tty.c > > Log Message: > use strlcpy() instead of strncpy() for gcc happiness > ... > @@ -1319,10 +1319,8 @@ terminal_settc(EditLine *el, int argc __ > if (argv == NULL || argv[1] == NULL || argv[2] == NULL) > return -1; > > - strncpy(what, ct_encode_string(argv[1], >el_scratch), sizeof(what)); > - what[sizeof(what) - 1] = '\0'; > - strncpy(how, ct_encode_string(argv[2], >el_scratch), sizeof(how)); > - how[sizeof(how) - 1] = '\0'; > + strlcpy(what, ct_encode_string(argv[1], >el_scratch), sizeof(what)); > + strlcpy(how, ct_encode_string(argv[2], >el_scratch), sizeof(how)); > This feels not good. strncpy->strlcpy has repercussions like how strlcpy doesn't zero out the remaining length and thus leaks uninitialized data. There has to be a reasonable way to handle these warnings instead of rototilling which str*cpy function is used.
Re: CVS commit: src/lib/libedit/TEST
On Jun 18, 7:49pm, m...@3am-software.com (Matt Thomas) wrote: -- Subject: Re: CVS commit: src/lib/libedit/TEST | | On Jun 18, 2014, at 1:12 PM, Christos Zoulas chris...@netbsd.org wrote: | | cast gotsig because it is long on some systems. | | Why cast to int instead of long? Because the only values we set it is 0 and 1, and that fits on an int? christos
Re: CVS commit: src/lib/libedit/TEST
On Jun 18, 2014, at 1:12 PM, Christos Zoulas chris...@netbsd.org wrote: cast gotsig because it is long on some systems. Why cast to int instead of long?
Re: CVS commit: src/lib/libedit
Am 19.05.14 21:54, schrieb Christos Zoulas:
Module Name: src
Committed By: christos
Date: Mon May 19 19:54:12 UTC 2014
Modified Files:
src/lib/libedit: tty.c tty.h
Log Message:
more tty modes refactoring, no functional change intended.
To generate a diff of this commit:
cvs rdiff -u -r1.43 -r1.44 src/lib/libedit/tty.c
cvs rdiff -u -r1.14 -r1.15 src/lib/libedit/tty.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/lib/libedit/tty.c
diff -u src/lib/libedit/tty.c:1.43 src/lib/libedit/tty.c:1.44
--- src/lib/libedit/tty.c:1.43Mon May 19 13:14:41 2014
+++ src/lib/libedit/tty.c Mon May 19 15:54:12 2014
@@ -1,4 +1,4 @@
-/* $NetBSD: tty.c,v 1.43 2014/05/19 17:14:41 christos Exp $*/
+/* $NetBSD: tty.c,v 1.44 2014/05/19 19:54:12 christos Exp $*/
/*-
* Copyright (c) 1992, 1993
@@ -37,7 +37,7 @@
#if 0
static char sccsid[] = @(#)tty.c8.1 (Berkeley) 6/4/93;
#else
-__RCSID($NetBSD: tty.c,v 1.43 2014/05/19 17:14:41 christos Exp $);
+__RCSID($NetBSD: tty.c,v 1.44 2014/05/19 19:54:12 christos Exp $);
#endif
#endif /* not lint not SCCSID */
@@ -919,6 +919,58 @@ tty_bind_char(EditLine *el, int force)
}
+private tcflag_t *
private? I think you mean static, right?
+tty__get_flag(struct termios *t, int kind) {
+ switch (kind) {
+ case MD_INP:
+ return t-c_iflag;
+ case MD_OUT:
+ return t-c_oflag;
+ case MD_CTL:
+ return t-c_cflag;
+ case MD_LIN:
+ return t-c_lflag;
+ default:
+ abort();
+ /*NOTREACHED*/
+ }
+}
+
+
+private tcflag_t
dito
+tty_update_flag(EditLine *el, tcflag_t f, int mode, int kind)
+{
+ f = ~el-el_tty.t_t[mode][kind].t_clrmask;
+ f |= el-el_tty.t_t[mode][kind].t_setmask;
+ return f;
+}
+
+
+private void
dito
+tty_update_flags(EditLine *el, int kind)
+{
+ tcflag_t *tt, *ed, *ex;
+ tt = tty__get_flag(el-el_tty.t_ts, kind);
+ ed = tty__get_flag(el-el_tty.t_ed, kind);
+ ex = tty__get_flag(el-el_tty.t_ex, kind);
+
+ if (*tt != *ex (kind != MD_CTL || *tt != *ed)) {
+ *ed = tty_update_flag(el, *tt, ED_IO, kind);
+ *ex = tty_update_flag(el, *tt, EX_IO, kind);
+ }
+}
+
+
+private void
dito
+tty_update_char(EditLine *el, int mode, int c) {
+ if (!((el-el_tty.t_t[mode][MD_CHAR].t_setmask C_SH(c)))
+ (el-el_tty.t_c[TS_IO][c] != el-el_tty.t_c[EX_IO][c]))
+ el-el_tty.t_c[mode][c] = el-el_tty.t_c[TS_IO][c];
+ if (el-el_tty.t_t[mode][MD_CHAR].t_clrmask C_SH(c))
+ el-el_tty.t_c[mode][c] = el-el_tty.t_vdisable;
+}
+
+
[...]
Christoph
Re: CVS commit: src/lib/libedit
In article 537a6540.1070...@gmx.de, Christoph Egger christoph_eg...@gmx.de wrote: private? I think you mean static, right? It does if compiled that way. Helps to read the rest of the file instead of just diffs :-) christos
Re: CVS commit: src/lib/libedit
On Wed, Aug 28, 2013 at 06:09:21PM +, Christos Zoulas wrote: Log Message: get rid of PATH_MAX. ...by leaking memory? How so? It probably uses less memory than before, but not on the bss but on the data segment. Sorry, misread. -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/lib/libedit
On Wed, Aug 28, 2013 at 04:05:21AM -0400, Christos Zoulas wrote: Modified Files: src/lib/libedit: readline.c Log Message: get rid of PATH_MAX. ...by leaking memory? -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/lib/libedit
Can we please stop abusing the notation of GCC is available to mean GCC is used? If you want a GCC-specific warning flag, at least put it into CWARNFLAGS.gcc. I'm actually in favour of dropping this unconditionally -- GCC 4.1 is just behaving way too lax for this option to make sense. Joerg On Sat, Jul 30, 2011 at 10:07:27AM +, Matthias Scheler wrote: Module Name: src Committed By: tron Date: Sat Jul 30 10:07:27 UTC 2011 Modified Files: src/lib/libedit: Makefile Log Message: Don't use -Wconversion with GCC 4.5 which will complain about all the expressions where signed variables are converted to unsigned in an expression e.g. size_t foo = sizeof(something) * int_var;. To generate a diff of this commit: cvs rdiff -u -r1.46 -r1.47 src/lib/libedit/Makefile Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Re: CVS commit: src/lib/libedit
On Fri, 29 Jul 2011, Christos Zoulas wrote: Log Message: add -Wunused-parameter Is that the right way? Perhaps WARNS=5? Or add -Wunused-parameter to WARNS=4. WARNS levels should be organised by the ease of avoiding or fixing the warnings; there's no need to add a new level for each warning. --apb (Alan Barrett)
Re: CVS commit: src/lib/libedit
In article 20110730064751.ga19...@apb-laptoy.apb.alt.za, Alan Barrett a...@cequrux.com wrote: On Fri, 29 Jul 2011, Christos Zoulas wrote: Log Message: add -Wunused-parameter Is that the right way? Perhaps WARNS=5? Or add -Wunused-parameter to WARNS=4. WARNS levels should be organised by the ease of avoiding or fixing the warnings; there's no need to add a new level for each warning. -Wunused-parameters and -Wconversion typically need annotations and casts to be fixed that are controvercial. Actually -Wconversion seems to be broken right now, as it complains about valid code. We should not be forcing them down people's throats. christos
Re: CVS commit: src/lib/libedit
In article 20110730205523.gb3...@britannica.bec.de, Joerg Sonnenberger jo...@britannica.bec.de wrote: Just cast the parameter to void? Do you prefer this than __unused? christos
Re: CVS commit: src/lib/libedit
On Sun, Jul 31, 2011 at 00:50:03 +, Christos Zoulas wrote: In article 20110730205523.gb3...@britannica.bec.de, Joerg Sonnenberger jo...@britannica.bec.de wrote: Just cast the parameter to void? Do you prefer this than __unused? I too would prefer something in the body of the function(wrapped in a macro to hide the gory details?). __unused in the signature just feels icky. -uwe
Re: CVS commit: src/lib/libedit
On Sun, 18 Apr 2010, Christos Zoulas wrote: Modified Files: src/lib/libedit: makelist Log Message: shame on solaris that is the last OS not supporting $() build.sh tries to set HOST_SH=/usr/xpg4/bin/sh on Solaris, and that shell does support $(...). Also, we make free use of $(...) elsewhere in the NetBSD build process, so I don't see why libedit/makelist should be an exception. --apb (Alan Barrett)
Re: CVS commit: src/lib/libedit
In article 20100419075526.ga1...@apb-laptoy.apb.alt.za, Alan Barrett a...@cequrux.com wrote: On Sun, 18 Apr 2010, Christos Zoulas wrote: Modified Files: src/lib/libedit: makelist Log Message: shame on solaris that is the last OS not supporting $() build.sh tries to set HOST_SH=/usr/xpg4/bin/sh on Solaris, and that shell does support $(...). Also, we make free use of $(...) elsewhere in the NetBSD build process, so I don't see why libedit/makelist should be an exception. It is for the portable libedit distribution that does not use build.sh, but autoconf. christos
Re: CVS commit: src/lib/libedit
hi, all.
i found one more big problem.
we had lost source/binary level backward compatibility, sigh.
el_set(3) defines EL_GETCFN flag, and some application(such as Asterisk) use it:
el_set()
Set editline parameters. op determines which parameter to set, and
each operation has its own parameter list.
The following values for op are supported, along with the required
argument list:
EL_GETCFN, int (*f)(EditLine *, char *c)
Define the character reading function as f, which is to
return the number of characters read and store them in c.
This function is called internally by el_gets() and
el_getc(). The builtin function can be set or restored with
the special function name ``EL_BUILTIN_GETCFN''.
but, this commit silently added following change(and it doesn't
expected by anyone)
int(*f)(EditLine *, char *) - int (*f)(EditLine *, wchar_t *)
so if old application binary link new libedit, it doesn't work anymore ;-.
[src/lib/libedit/el.c]
281 case EL_GETCFN:
282 {
283 el_rfunc_t rc = va_arg(ap, el_rfunc_t);
284 rv = el_read_setfn(el, rc);
285 el-el_flags = ~NARROW_READ;
286 break;
287 }
[src/lib/libedit/read.h]
@@ -35,7 +35,7 @@
#ifndef_h_el_read
#define_h_el_read
-typedef int (*el_rfunc_t)(EditLine *, char *);
+typedef int (*el_rfunc_t)(EditLine *, Char *);
typedef struct el_read_t {
el_rfunc_t read_char; /* Function to read a character */
what shoulld we do to solve this problem?
i believe tempolary backout this patch is the way we have to go.
# i'm now started completely rewriting wide-character support for libedit.
# but it still need lots of time.
very truly yours.
--
Takehiko NOZAKItakehiko.noz...@gmail.com
2010/1/14 Takehiko NOZAKI takehiko.noz...@gmail.com:
hi, all.
i found following problems this patch:
1. don't write UTF-8 locale dependent ``cheat'' code in locale
independent libedit,
such as enc_width(), utf8_islead() and so on, completely meaningless.
our locale implementation is CodeSet Independ policy, wchar_t != UCS4.
please read itojun@'s paper.
http://www.usenix.org/events/usenix01/freenix01/hagino.html
2. cast wchar_t - wint_t in the argument of isw* function.
consider sizeof(wchar_t) sizeof(wint_t) case (for integer promotion).
3. kill evil __STDC_ISO_10646__ usage.
if you want kick some encodings not compatible with ASCII(like EBCDIC),
use __STDC_MB_MIGHT_NEQ_WC__ instead.
see http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_321.htm
4. library function must not use thread unsafe version of mb/wc conversion,
like mbtowc, wctomb and so on. use mbrtowc/wcrtomb instead.
this may broke compatibility some old apllication that uses mbtowc/wctomb.
i'm willing to fix them, but i'm busy now, not enough time to do it til
April...
very truly yours.
--
Takehiko NOZAKItakehiko.noz...@gmail.com
Re: CVS commit: src/lib/libedit
On Mar 7, 5:24am, takehiko.noz...@gmail.com (Takehiko NOZAKI) wrote:
-- Subject: Re: CVS commit: src/lib/libedit
| -typedef int (*el_rfunc_t)(EditLine *, char *);
| +typedef int (*el_rfunc_t)(EditLine *, Char *);
|
| typedef struct el_read_t {
| el_rfunc_t read_char; /* Function to read a character */
|
|
| what shoulld we do to solve this problem?
| i believe tempolary backout this patch is the way we have to go.
Are you sure? because the code handles the narrow case?
(That is what the NARROW_READ flag is for.
| # i'm now started completely rewriting wide-character support for libedit.
| # but it still need lots of time.
Good luck.
christos
Re: CVS commit: src/lib/libedit
oh, i see. i don't recognize NARROW_READ flag.
source/binary compatibility are still **kept**,
i withdrawn previous mail, sorry.
BTW, this hack is not good for MB_CUR_MAX 1 case.
if el_set(EL_GETCFN) called without EL_BUILTIN_GETCFN,
we can't treat multibyte correctly.
# because where we convert multibyte - wide-character is
# EL_BUILTIN_GETCFN(=read_char) itself.
very truly yours.
--
Takehiko NOZAKItakehiko.noz...@gmail.com
2010/3/7 Christos Zoulas chris...@zoulas.com:
On Mar 7, 5:24am, takehiko.noz...@gmail.com (Takehiko NOZAKI) wrote:
-- Subject: Re: CVS commit: src/lib/libedit
| -typedef int (*el_rfunc_t)(EditLine *, char *);
| +typedef int (*el_rfunc_t)(EditLine *, Char *);
|
| typedef struct el_read_t {
| el_rfunc_t read_char; /* Function to read a character */
|
|
| what shoulld we do to solve this problem?
| i believe tempolary backout this patch is the way we have to go.
Are you sure? because the code handles the narrow case?
(That is what the NARROW_READ flag is for.
| # i'm now started completely rewriting wide-character support for libedit.
| # but it still need lots of time.
Good luck.
christos
Re: CVS commit: src/lib/libedit
On Mar 7, 7:46am, takehiko.noz...@gmail.com (Takehiko NOZAKI) wrote: -- Subject: Re: CVS commit: src/lib/libedit | oh, i see. i don't recognize NARROW_READ flag. | source/binary compatibility are still **kept**, | i withdrawn previous mail, sorry. | | | BTW, this hack is not good for MB_CUR_MAX 1 case. | if el_set(EL_GETCFN) called without EL_BUILTIN_GETCFN, | we can't treat multibyte correctly. | # because where we convert multibyte - wide-character is | # EL_BUILTIN_GETCFN(=3Dread_char) itself. Totally agree, but there is other breakage for the non UTF8 and MB_CUR_MAX 1 case. christos
Re: CVS commit: src/lib/libedit
hi, can you please don't hardcode utf-8? YAMAMOTO Takashi Module Name: src Committed By: christos Date: Wed Dec 30 22:37:40 UTC 2009 Modified Files: src/lib/libedit: Makefile chared.c chared.h common.c el.c el.h emacs.c filecomplete.c filecomplete.h hist.c hist.h histedit.h history.c key.c key.h makelist map.c map.h parse.c parse.h prompt.c prompt.h read.c read.h readline.c refresh.c refresh.h search.c search.h sys.h term.c term.h tokenizer.c tty.c tty.h vi.c Added Files: src/lib/libedit: chartype.c chartype.h eln.c Log Message: Wide character support (UTF-8) from Johny Mattsson; currently disabled. To generate a diff of this commit: cvs rdiff -u -r1.37 -r1.38 src/lib/libedit/Makefile cvs rdiff -u -r1.27 -r1.28 src/lib/libedit/chared.c cvs rdiff -u -r1.18 -r1.19 src/lib/libedit/chared.h src/lib/libedit/el.h cvs rdiff -u -r0 -r1.1 src/lib/libedit/chartype.c src/lib/libedit/chartype.h \ src/lib/libedit/eln.c cvs rdiff -u -r1.23 -r1.24 src/lib/libedit/common.c cvs rdiff -u -r1.55 -r1.56 src/lib/libedit/el.c cvs rdiff -u -r1.22 -r1.23 src/lib/libedit/emacs.c src/lib/libedit/key.c \ src/lib/libedit/parse.c cvs rdiff -u -r1.16 -r1.17 src/lib/libedit/filecomplete.c \ src/lib/libedit/prompt.c cvs rdiff -u -r1.8 -r1.9 src/lib/libedit/filecomplete.h src/lib/libedit/map.h \ src/lib/libedit/search.h cvs rdiff -u -r1.15 -r1.16 src/lib/libedit/hist.c src/lib/libedit/tokenizer.c cvs rdiff -u -r1.10 -r1.11 src/lib/libedit/hist.h cvs rdiff -u -r1.41 -r1.42 src/lib/libedit/histedit.h cvs rdiff -u -r1.34 -r1.35 src/lib/libedit/history.c \ src/lib/libedit/refresh.c cvs rdiff -u -r1.12 -r1.13 src/lib/libedit/key.h src/lib/libedit/makelist \ src/lib/libedit/sys.h cvs rdiff -u -r1.24 -r1.25 src/lib/libedit/map.c cvs rdiff -u -r1.6 -r1.7 src/lib/libedit/parse.h src/lib/libedit/read.h cvs rdiff -u -r1.9 -r1.10 src/lib/libedit/prompt.h cvs rdiff -u -r1.52 -r1.53 src/lib/libedit/read.c cvs rdiff -u -r1.85 -r1.86 src/lib/libedit/readline.c cvs rdiff -u -r1.5 -r1.6 src/lib/libedit/refresh.h cvs rdiff -u -r1.21 -r1.22 src/lib/libedit/search.c cvs rdiff -u -r1.56 -r1.57 src/lib/libedit/term.c cvs rdiff -u -r1.20 -r1.21 src/lib/libedit/term.h cvs rdiff -u -r1.31 -r1.32 src/lib/libedit/tty.c cvs rdiff -u -r1.11 -r1.12 src/lib/libedit/tty.h cvs rdiff -u -r1.30 -r1.31 src/lib/libedit/vi.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Re: CVS commit: src/lib/libedit
On Jan 13, 8:21am, y...@mwd.biglobe.ne.jp (YAMAMOTO Takashi) wrote: -- Subject: Re: CVS commit: src/lib/libedit | hi, | | can you please don't hardcode utf-8? | | YAMAMOTO Takashi Once we verify that it works on non utf-8, sure. christos
Re: CVS commit: src/lib/libedit
On Jan 13, 8:21am, y...@mwd.biglobe.ne.jp (YAMAMOTO Takashi) wrote: -- Subject: Re: CVS commit: src/lib/libedit | hi, | | can you please don't hardcode utf-8? | | YAMAMOTO Takashi Once we verify that it works on non utf-8, sure. what do you mean by verify? YAMAMOTO Takashi christos
re: CVS commit: src/lib/libedit
Module Name: src Committed By:christos Date:Mon Sep 7 21:24:34 UTC 2009 Modified Files: src/lib/libedit: histedit.h history.c readline.c src/lib/libedit/readline: readline.h Log Message: apply apple patches from: http://opensource.apple.com/source/libedit/libedit-11/patches/ could you please describe the changes themselves, rather than the source of them? thanks, .mrg.
