[patch] nm: some code cleanup
Some code cleanup:
- disable elf_load_phdrs() and elf_fix_phdrs() functions as there
aren't in used. I also add a comment for elf_load_phdrs(): the code
seems as buggy as elf_load_shdrs() against crafted input.
Some other code could be dead too: I have not investigated a lot in
this direction for now.
- change some if(x) free(x) idioms to free(x)
--
Sébastien Marie
Index: elf.c
===
RCS file: /cvs/src/usr.bin/nm/elf.c,v
retrieving revision 1.28
diff -u -p -r1.28 elf.c
--- elf.c 17 May 2015 20:19:08 - 1.28
+++ elf.c 19 Jun 2015 06:48:48 -
@@ -170,6 +170,8 @@ elf_load_shdrs(const char *name, FILE *f
return (shdr);
}
+#if 0
+/* XXX: elf_load_phdrs need code review against crafted input */
Elf_Phdr *
elf_load_phdrs(const char *name, FILE *fp, off_t foff, Elf_Ehdr *head)
{
@@ -195,6 +197,7 @@ elf_load_phdrs(const char *name, FILE *f
elf_fix_phdrs(head, phdr);
return (phdr);
}
+#endif
int
elf_fix_shdrs(Elf_Ehdr *eh, Elf_Shdr *shdr)
@@ -221,6 +224,7 @@ elf_fix_shdrs(Elf_Ehdr *eh, Elf_Shdr *sh
return (1);
}
+#if 0
int
elf_fix_phdrs(Elf_Ehdr *eh, Elf_Phdr *phdr)
{
@@ -243,6 +247,7 @@ elf_fix_phdrs(Elf_Ehdr *eh, Elf_Phdr *ph
return (1);
}
+#endif
int
elf_fix_sym(Elf_Ehdr *eh, Elf_Sym *sym)
@@ -562,10 +567,8 @@ elf_symload(const char *name, FILE *fp,
free(shstr);
if (stab == NULL) {
warnx("%s: no name list", name);
- if (*pnames)
- free(*pnames);
- if (*psnames)
- free(*psnames);
+ free(*pnames);
+ free(*psnames);
return (1);
}
Index: elfuncs.h
===
RCS file: /cvs/src/usr.bin/nm/elfuncs.h,v
retrieving revision 1.3
diff -u -p -r1.3 elfuncs.h
--- elfuncs.h 30 Sep 2006 14:34:13 - 1.3
+++ elfuncs.h 19 Jun 2015 06:48:48 -
@@ -30,9 +30,9 @@ extern char *stab;
intelf32_fix_header(Elf32_Ehdr *eh);
Elf32_Shdr*elf32_load_shdrs(const char *, FILE *, off_t, Elf32_Ehdr *);
-Elf32_Phdr*elf32_load_phdrs(const char *, FILE *, off_t, Elf32_Ehdr *);
+/*Elf32_Phdr*elf32_load_phdrs(const char *, FILE *, off_t, Elf32_Ehdr *);*/
intelf32_fix_shdrs(Elf32_Ehdr *eh, Elf32_Shdr *shdr);
-intelf32_fix_phdrs(Elf32_Ehdr *eh, Elf32_Phdr *phdr);
+/*int elf32_fix_phdrs(Elf32_Ehdr *eh, Elf32_Phdr *phdr);*/
intelf32_fix_sym(Elf32_Ehdr *eh, Elf32_Sym *sym);
intelf32_size(Elf32_Ehdr *, Elf32_Shdr *, u_long *, u_long *, u_long *);
intelf32_symloadx(const char *, FILE *, off_t, Elf32_Ehdr *, Elf32_Shdr *,
@@ -43,9 +43,9 @@ int elf32_symload(const char *, FILE *,
intelf64_fix_header(Elf64_Ehdr *eh);
Elf64_Shdr*elf64_load_shdrs(const char *, FILE *, off_t, Elf64_Ehdr *);
-Elf64_Phdr*elf64_load_phdrs(const char *, FILE *, off_t, Elf64_Ehdr *);
+/*Elf64_Phdr*elf64_load_phdrs(const char *, FILE *, off_t, Elf64_Ehdr *);*/
intelf64_fix_shdrs(Elf64_Ehdr *eh, Elf64_Shdr *shdr);
-intelf64_fix_phdrs(Elf64_Ehdr *eh, Elf64_Phdr *phdr);
+/*int elf64_fix_phdrs(Elf64_Ehdr *eh, Elf64_Phdr *phdr);*/
intelf64_fix_sym(Elf64_Ehdr *eh, Elf64_Sym *sym);
intelf64_size(Elf64_Ehdr *, Elf64_Shdr *, u_long *, u_long *, u_long *);
intelf64_symloadx(const char *, FILE *, off_t, Elf64_Ehdr *, Elf64_Shdr *,
[patch] nm: on error, set globals to NULL
Hi,
This patch ensure that when an error is detected, the freed variables in
elf_symloadx() are reinitialised.
Else show_file() in nm.c will used these variables, even if they has
just been freed. (nm.c +689).
Problem found by afl.
--
Sébastien Marie
Index: elf.c
===
RCS file: /cvs/src/usr.bin/nm/elf.c,v
retrieving revision 1.28
diff -u -p -r1.28 elf.c
--- elf.c 17 May 2015 20:19:08 - 1.28
+++ elf.c 19 Jun 2015 06:42:12 -
@@ -479,6 +479,7 @@ elf_symloadx(const char *name, FILE *fp,
warn("%s: malloc names", name);
if (stab)
MUNMAP(stab, *pstabsize);
+ *pnrawnames = 0;
return (1);
}
if ((*psnames = calloc(*pnrawnames, sizeof(np))) ==
NULL) {
@@ -486,6 +487,8 @@ elf_symloadx(const char *name, FILE *fp,
if (stab)
MUNMAP(stab, *pstabsize);
free(*pnames);
+ *pnames = NULL;
+ *pnrawnames = 0;
return (1);
}
@@ -497,6 +500,9 @@ elf_symloadx(const char *name, FILE *fp,
MUNMAP(stab, *pstabsize);
free(*pnames);
free(*psnames);
+ *pnames = NULL;
+ *psnames = NULL;
+ *pnrawnames = 0;
return (1);
}
Index: util.h
===
RCS file: /cvs/src/usr.bin/nm/util.h,v
retrieving revision 1.3
diff -u -p -r1.3 util.h
--- util.h 17 May 2015 20:19:08 - 1.3
+++ util.h 19 Jun 2015 06:42:12 -
@@ -26,6 +26,7 @@
munmap(addr, len); \
else\
free(addr); \
+ addr = NULL;\
} while (0)
extern int usemmap;
[patch] nm: minimal section header size
Hi,
This patch ensure that e_shentsize (sections header's size in bytes) is
big enough to fill at least one Elf_Shdr.
Please note, I am not completely sure to have understand this part:
calloc() is called with a dynamic element size (e_shentsize: readed for
file), but the variable is declared as Elf_Shdr[] (array of Elf_Shdr:
which is a fixed element size).
While here, inverts calloc() arguments to be calloc(nmemb, size),
according to fread() call after.
This problem was found with afl, when e_shentsize was 1.
--
Sébastien Marie
Index: b/usr.bin/nm/elf.c
===
--- a/usr.bin/nm/elf.c 2015-06-19 06:26:13.704127213 +0200
+++ b/usr.bin/nm/elf.c 2015-06-19 06:48:58.806582243 +0200
@@ -159,7 +159,12 @@
return (NULL);
}
- if ((shdr = calloc(head->e_shentsize, head->e_shnum)) == NULL) {
+ if (head->e_shentsize < sizeof(Elf_Shdr)) {
+ warnx("%s: inconsistent section header size", name);
+ return (NULL);
+ }
+
+ if ((shdr = calloc(head->e_shnum, head->e_shentsize)) == NULL) {
warn("%s: malloc shdr", name);
return (NULL);
}
[patch] nm: read after bound
Hi,
This patch corrects a read after bound that occurs in strcmp (line just
after the added bound check).
Found with afl.
--
Sébastien Marie
Index: elf.c
===
RCS file: /cvs/src/usr.bin/nm/elf.c,v
retrieving revision 1.28
diff -u -p -r1.28 elf.c
--- elf.c 17 May 2015 20:19:08 - 1.28
+++ elf.c 17 Jun 2015 15:18:03 -
@@ -441,7 +451,7 @@ elf_size(Elf_Ehdr *head, Elf_Shdr *shdr,
int
elf_symloadx(const char *name, FILE *fp, off_t foff, Elf_Ehdr *eh,
-Elf_Shdr *shdr, char *shstr, struct nlist **pnames,
+Elf_Shdr *shdr, char *shstr, long shstrsize, struct nlist **pnames,
struct nlist ***psnames, size_t *pstabsize, int *pnrawnames,
const char *strtab, const char *symtab)
{
@@ -451,6 +461,10 @@ elf_symloadx(const char *name, FILE *fp,
int i;
for (i = 0; i < eh->e_shnum; i++) {
+ if (shdr[i].sh_name >= shstrsize) {
+ warnx("%s: corrupt file", name);
+ return (1);
+ }
if (!strcmp(shstr + shdr[i].sh_name, strtab)) {
*pstabsize = shdr[i].sh_size;
if (*pstabsize > SIZE_MAX) {
@@ -551,11 +565,11 @@ elf_symload(const char *name, FILE *fp,
stab = NULL;
*pnames = NULL; *psnames = NULL; *pnrawnames = 0;
if (!dynamic_only) {
- elf_symloadx(name, fp, foff, eh, shdr, shstr, pnames,
+ elf_symloadx(name, fp, foff, eh, shdr, shstr, shstrsize, pnames,
psnames, pstabsize, pnrawnames, ELF_STRTAB, ELF_SYMTAB);
}
if (stab == NULL) {
- elf_symloadx(name, fp, foff, eh, shdr, shstr, pnames,
+ elf_symloadx(name, fp, foff, eh, shdr, shstr, shstrsize, pnames,
psnames, pstabsize, pnrawnames, ELF_DYNSTR, ELF_DYNSYM);
}
Index: elfuncs.h
===
RCS file: /cvs/src/usr.bin/nm/elfuncs.h,v
retrieving revision 1.3
diff -u -p -r1.3 elfuncs.h
--- elfuncs.h 30 Sep 2006 14:34:13 - 1.3
+++ elfuncs.h 17 Jun 2015 15:18:03 -
@@ -36,7 +36,7 @@ int elf32_fix_phdrs(Elf32_Ehdr *eh, Elf3
intelf32_fix_sym(Elf32_Ehdr *eh, Elf32_Sym *sym);
intelf32_size(Elf32_Ehdr *, Elf32_Shdr *, u_long *, u_long *, u_long *);
intelf32_symloadx(const char *, FILE *, off_t, Elf32_Ehdr *, Elf32_Shdr *,
- char *, struct nlist **, struct nlist ***, size_t *, int *,
+ char *, long, struct nlist **, struct nlist ***, size_t *, int *,
const char *, const char *);
intelf32_symload(const char *, FILE *, off_t, Elf32_Ehdr *, Elf32_Shdr *,
struct nlist **, struct nlist ***, size_t *, int *);
@@ -49,7 +49,7 @@ int elf64_fix_phdrs(Elf64_Ehdr *eh, Elf6
intelf64_fix_sym(Elf64_Ehdr *eh, Elf64_Sym *sym);
intelf64_size(Elf64_Ehdr *, Elf64_Shdr *, u_long *, u_long *, u_long *);
intelf64_symloadx(const char *, FILE *, off_t, Elf64_Ehdr *, Elf64_Shdr *,
- char *, struct nlist **, struct nlist ***, size_t *, int *,
+ char *, long, struct nlist **, struct nlist ***, size_t *, int *,
const char *, const char *);
intelf64_symload(const char *, FILE *, off_t, Elf64_Ehdr *, Elf64_Shdr *,
struct nlist **, struct nlist ***, size_t *, int *);
Re: [patch] nm segfault
On Wed, Jun 17, 2015 at 02:43:41PM +0200, Sébastien Marie wrote:
> Hi,
>
> I would like to report a SEGFAULT in nm(1) that occurs with object-file
> with no section headers (e_shnum = 0).
>
>
> Index: elf.c
> ===
> RCS file: /cvs/src/usr.bin/nm/elf.c,v
> retrieving revision 1.28
> diff -u -p -r1.28 elf.c
> --- elf.c 17 May 2015 20:19:08 - 1.28
> +++ elf.c 17 Jun 2015 12:05:25 -
> @@ -149,6 +149,16 @@ elf_load_shdrs(const char *name, FILE *f
>
> elf_fix_header(head);
>
> + if (head->e_shnum == 0) {
> + warnx("%s: no section header table", name);
> + return (NULL);
> + }
> +
> + if (head->e_shstrndx >= head->e_shentsize * head->e_shnum) {
> + warnx("%s: inconsistent section header table", name);
> + return (NULL);
> + }
wrong here: the check should be (head->e_shstrndx >= head->e_shnum)
corrected patch below.
> if ((shdr = calloc(head->e_shentsize, head->e_shnum)) == NULL) {
> warn("%s: malloc shdr", name);
> return (NULL);
--
Sébastien Marie
Index: elf.c
===
RCS file: /cvs/src/usr.bin/nm/elf.c,v
retrieving revision 1.28
diff -u -p -r1.28 elf.c
--- elf.c 17 May 2015 20:19:08 - 1.28
+++ elf.c 17 Jun 2015 15:07:19 -
@@ -149,6 +149,16 @@ elf_load_shdrs(const char *name, FILE *f
elf_fix_header(head);
+ if (head->e_shnum == 0) {
+ warnx("%s: no section header table", name);
+ return (NULL);
+ }
+
+ if (head->e_shstrndx >= head->e_shnum) {
+ warnx("%s: inconsistent section header table", name);
+ return (NULL);
+ }
+
if ((shdr = calloc(head->e_shentsize, head->e_shnum)) == NULL) {
warn("%s: malloc shdr", name);
return (NULL);
[patch] nm segfault
Hi,
I would like to report a SEGFAULT in nm(1) that occurs with object-file
with no section headers (e_shnum = 0). This object-file was generated by
eg++ (I am not sure if the object-file is valid or not).
I am also able to reproduce the problem with edited elf object (using
hte [editors/ht]) by manually setting the section header count to zero.
$ readelf -h obj/cciVHsvd-2.o
ELF Header:
Magic: 7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00
Class: ELF64
Data: 2's complement, little endian
Version: 1 (current)
OS/ABI:UNIX - System V
ABI Version: 0
Type: REL (Relocatable file)
Machine: Advanced Micro Devices X86-64
Version: 0x1
Entry point address: 0x0
Start of program headers: 0 (bytes into file)
Start of section headers: 8932544 (bytes into file)
Flags: 0x0
Size of this header: 64 (bytes)
Size of program headers: 0 (bytes)
Number of program headers: 0
Size of section headers: 64 (bytes)
Number of section headers: 0 (73122)
Section header string table index: 65535 (73374)
$ gdb -q -args ./obj/nm -n obj/cciVHsvd-2.o
(gdb) r
Starting program: /usr/obj/usr.bin/nm/nm -n obj/cciVHsvd-2.o
Program received signal SIGSEGV, Segmentation fault.
0x19a9f830728e in elf64_symload (name=0x7f7efe82 "obj/cciVHsvd-2.o",
fp=0x19aca16c7180, foff=0, eh=0x7f7efbb0,
shdr=0x19aca4026720, pnames=0x7f7efb38, psnames=0x7f7efb30,
pstabsize=0x7f7efb28, pnrawnames=0x7f7efb70) at elf64.c:529
529 shstrsize = shdr[eh->e_shstrndx].sh_size;
(gdb) bt
#0 0x19a9f830728e in elf64_symload (name=0x7f7efe82
"obj/cciVHsvd-2.o", fp=0x19aca16c7180, foff=0, eh=0x7f7efbb0,
shdr=0x19aca4026720, pnames=0x7f7efb38, psnames=0x7f7efb30,
pstabsize=0x7f7efb28, pnrawnames=0x7f7efb70) at elf64.c:529
#1 0x19a9f83037ca in show_file (count=1, warn_fmt=1, name=0x7f7efe82
"obj/cciVHsvd-2.o", fp=0x19aca16c7180, foff=0,
head=0x7f7efbb0) at /usr/src/usr.bin/nm/nm.c:636
#2 0x19a9f8302548 in process_file (count=1, fname=0x7f7efe82
"obj/cciVHsvd-2.o") at /usr/src/usr.bin/nm/nm.c:267
#3 0x19a9f83022e3 in main (argc=1, argv=0x7f7efcd8) at
/usr/src/usr.bin/nm/nm.c:208
(gdb) print eh
$1 = (Elf64_Ehdr *) 0x7f7efbb0
(gdb) print *eh
$2 = {e_ident = "\177ELF\002\001\001\000\000\000\000\000\000\000\000", e_type =
1, e_machine = 62, e_version = 1, e_entry = 0, e_phoff = 0,
e_shoff = 8932544, e_flags = 0, e_ehsize = 64, e_phentsize = 0, e_phnum = 0,
e_shentsize = 64, e_shnum = 0, e_shstrndx = 65535}
(gdb) print shdr
$3 = (Elf64_Shdr *) 0x19aca4026720
(gdb) print *shdr
Cannot access memory at address 0x19aca4026720
(gdb) quit
The segfault is caused by "shdr[eh->e_shstrndx]" (src/usr.bin/nm/elf.c:528).
shdr is allocated by elf_load_shdrs() (elf.c:152):
>if ((shdr = calloc(head->e_shentsize, head->e_shnum)) == NULL) {
> warn("%s: malloc shdr", name);
> return (NULL);
> }
Here, the object-file has e_shnum=0 (no section header table), so shdr
is an zero sized object.
The patch adds two check:
- e_shnum == 0: no section header table
- a consistency check (should prevent craft object-file to generate
out-of-bound read).
Maybe a check for overflow would be needed too ?
--
Sébastien Marie
Index: elf.c
===
RCS file: /cvs/src/usr.bin/nm/elf.c,v
retrieving revision 1.28
diff -u -p -r1.28 elf.c
--- elf.c 17 May 2015 20:19:08 - 1.28
+++ elf.c 17 Jun 2015 12:05:25 -
@@ -149,6 +149,16 @@ elf_load_shdrs(const char *name, FILE *f
elf_fix_header(head);
+ if (head->e_shnum == 0) {
+ warnx("%s: no section header table", name);
+ return (NULL);
+ }
+
+ if (head->e_shstrndx >= head->e_shentsize * head->e_shnum) {
+ warnx("%s: inconsistent section header table", name);
+ return (NULL);
+ }
+
if ((shdr = calloc(head->e_shentsize, head->e_shnum)) == NULL) {
warn("%s: malloc shdr", name);
return (NULL);
roadmap for libc/locale
Hi, I think I didn't do the things in right order. I started to submit small commits, but without a global view of the direction taken it is useless. So I would like to present a roadmap in my intended work in libc locale area. If some developers could help or guide me with comments or suggestions, it would be great. First, why ? I would like to port libc++ (http://libcxx.llvm.org) to OpenBSD, in order to be able to have a llvm toolchain in ports that don't depend on gcc 4.9 (gcc-4.2 in base is unable to build recent llvm due to lack of c++11 support). We could build recent llvm (3.6.1) using gcc-4.9, but as a c++11 runtime is needed, the resulting binary is linked to libestdc++ (gcc-libs package). What is the relationship with libc/locale ? libc++ needs some POSIX functions in locale area that are missing in OpenBSD. These functions are uselocale(3), newlocale(3) and freelocale(3). (see http://pubs.opengroup.org/onlinepubs/9699919799/functions/newlocale.html for details). Basically it is per-thread support of locale. The intended work in libc/locale is to add these functions, and the related stuff only (locale_t type, ctype *_l functions [like isalnum_l]). My starting point is to considere global variables used in current code. Some are part of locale state (like current_categories in setlocale.c), others are temporary storage (like new_categories in setlocale.c). Temporary storage should be keep as local variable (for thread-safetly), whereas locale state variable should be transfered to locale_t (opaque type, internally a pointer to struct _locale_t). The global variables affected by setlocale(3) function are: - current_categories (locale/setlocale.c) - _CurrentRuneLocale (locale/runetable.c) - __mb_cur_max (locale/__mb_cur_max.c) - _ctype_ (gen/ctype_.c) - _toupper_tab_ (gen/toupper_.c) - _tolower_tab_ (gen/tolower_.c) Others global variables in locale area: - _CurrentMessagesLocale - _CurrentMonetaryLocale - _CurrentNumericLocale - _CurrentTimeLocale All of these should be part of locale_t, in order to be accessed in thread-safe manner, and to be able to be setted by newlocale(3) (the thread-safe counterpart of setlocale(3)). The second list (_CurrentMessagesLocale to _CurrentTimeLocale) is subject to discussion if it need to be part of _locale_t struct as these variables seems to be not modified. Some others elements need to be part of struct _locale_t: - magic_header: uselocale(3) need to be able to reject invalid locale object - marker to known if the object is installed as locale or not (created, but not passed to uselocale(3)). It would permit to manage ressource liberation in newlocale(3). A pivot function __get_locale() would be used to internally retrieve: - the global locale-state: struct _locale_t _locale_global_locale (and locale_t LC_GLOBAL_LOCALE = &_locale_global_locale) - the per-thread locale-state (if setted by uselocale(3)). This function would be weak to have different code if linked with pthread or not. The global state (_locale_global_locale) could be directly used internally by not thread-safe functions (like setlocale()). Others places should use __getlocale()->... idiom in order to retrieve the proper locale-state (global or per-thread state) in thread-safe manner. A rewrite of severals functions will be done, in order to transfert code in thread-safe functions, and call these functions with _locale_global_locale (or LC_GLOBAL_LOCALE). For example, having isalnum(3) based on isalnum_l(3), and isalnum_l() with mostly the code of isalnum(). I hope my explanation is understandable enough. Thanks. -- Sébastien Marie
patch: libc/locale/setlocale.c new_categories variable (3 and last)
Last patch for removing new_categories as global variable. The variable is now (after patch 1 and 2) only used in setlocale() function. The variable content isn't used outside the function, as the content is copied from new_categories to current_categories variable. The variable could be passed from global to local variable of setlocale(). -- Sébastien Marie Index: locale/setlocale.c === RCS file: /cvs/src/lib/libc/locale/setlocale.c,v retrieving revision 1.21 diff -u -p -r1.21 setlocale.c --- locale/setlocale.c 9 Jun 2015 20:04:04 - 1.21 +++ locale/setlocale.c 12 Jun 2015 13:56:16 - @@ -68,17 +68,12 @@ static char current_categories[_LC_LAST] "C" }; -/* - * The locales we are going to try and load - */ -static char new_categories[_LC_LAST][32]; - static char current_locale_string[_LC_LAST * 33]; static char*currentlocale(void); static void revert_to_default(int); static int load_locale_sub(int, const char *); -static char*loadlocale(int); +static char*loadlocale(int, const char *); static const char *__get_locale_env(int); char * @@ -87,6 +82,7 @@ setlocale(int category, const char *loca int i, loadlocale_success; size_t len; const char *env, *r; + char new_categories[_LC_LAST][32]; if (category < 0 || category >= _LC_LAST) return (NULL);
patch: libc/locale/setlocale.c new_categories variable (1)
I would like to remove new_categories variable from global.
This variable is used as temporary buffer in order to do checking
before copying the content to current_categories variable.
This first patch just remove the use of new_categories in load_locale_sub()
function.
load_locale_sub() is called only at one place in loadlocale():
if (!load_locale_sub(category, new_categories[category])) {
The second argument (locname) of load_locale_sub() is always defined
with new_categories[category].
So we could replace new_categories[category] by locname in function body.
--
Sébastien Marie
Index: locale/setlocale.c
===
RCS file: /cvs/src/lib/libc/locale/setlocale.c,v
retrieving revision 1.21
diff -u -p -r1.21 setlocale.c
--- locale/setlocale.c 9 Jun 2015 20:04:04 - 1.21
+++ locale/setlocale.c 12 Jun 2015 13:32:08 -
@@ -241,8 +241,7 @@ static int
load_locale_sub(int category, const char *locname)
{
/* check for the default locales */
- if (!strcmp(new_categories[category], "C") ||
- !strcmp(new_categories[category], "POSIX")) {
+ if (!strcmp(locname, "C") || !strcmp(locname, "POSIX")) {
revert_to_default(category);
return 0;
}
patch: libc/locale/setlocale.c new_categories variable (2)
Here a second patch which remove the use of new_categories from
loadlocale() function.
loadlocate() use only one element from new_categories: the current
category we want to assign to current_categories[category].
So instead of using new_categories globally, we can pass as argument the
locale name (locname), and used it in loadlocate() body for:
- check if the wanted value is already set
- effectively load the locale (load_locale_sub call)
- effectively assign the value in current_categories
--
Sébastien Marie
Index: locale/setlocale.c
===
RCS file: /cvs/src/lib/libc/locale/setlocale.c,v
retrieving revision 1.21
diff -u -p -r1.21 setlocale.c
--- locale/setlocale.c 9 Jun 2015 20:04:04 - 1.21
+++ locale/setlocale.c 12 Jun 2015 13:46:40 -
@@ -78,7 +78,7 @@ static char current_locale_string[_LC_LA
static char*currentlocale(void);
static void revert_to_default(int);
static int load_locale_sub(int, const char *);
-static char*loadlocale(int);
+static char*loadlocale(int, const char *);
static const char *__get_locale_env(int);
char *
@@ -153,11 +153,11 @@ setlocale(int category, const char *loca
}
if (category)
- return (loadlocale(category));
+ return (loadlocale(category, new_categories[category]));
loadlocale_success = 0;
for (i = 1; i < _LC_LAST; ++i) {
- if (loadlocale(i) != NULL)
+ if (loadlocale(i, new_categories[i]) != NULL)
loadlocale_success = 1;
}
@@ -272,15 +271,14 @@ load_locale_sub(int category, const char
}
static char *
-loadlocale(int category)
+loadlocale(int category, const char *locname)
{
- if (strcmp(new_categories[category],
- current_categories[category]) == 0)
+ if (strcmp(locname, current_categories[category]) == 0)
return (current_categories[category]);
- if (!load_locale_sub(category, new_categories[category])) {
+ if (!load_locale_sub(category, locname)) {
(void)strlcpy(current_categories[category],
- new_categories[category],
sizeof(current_categories[category]));
+ locname, sizeof(current_categories[category]));
return current_categories[category];
} else {
return NULL;
Re: patch: catopen(3), locale setting and getenv
On Fri, Jun 12, 2015 at 10:35:02AM +0200, Stefan Sperling wrote: > On Fri, Jun 12, 2015 at 08:59:14AM +0200, Sébastien Marie wrote: > > If this change is desirable, I will propose patchs for programs in base > > in order to call setlocale(LC_ALL, "") at program initilisation. > > Calling setlocale() with LC_ALL will have other side effects apart from > messages. I think, for now, we should only call setlocale() in programs > that have been made fully local-aware. For example, calling setlocale() > while not making the application handle multi-byte sequences properly > might not be such a good idea. Yes, it is true. It would be a problem. > Localising error messages is not all that important at present. What's much > more important, for example, is interactive editing of UTF-8 in the shell and > in editors. Before that works, extensive localisation is rather pointless. > Currently, localisation and multibyte only work with tools installed from > ports, with very few exceptions (e.g. tmux and less). > > catopen() is an interface which isn't widely used. It seems to be an old relic > of posix. So I don't see much point in extending it. Nowadays, everyone uses > gettext() for localisation instead. strerror(3) implementation use catopen(), and strerror(3) (or others errno related error messages, like err(3) or warn(3)) are used a lot in base. > Whether or not full message localisation of the base system should happen > at all is likely a contentious question :-) The purpose of the patch wasn't to start a full message localisation of base system... :) I start reviewing locale, and while I was testing in regress, I was suprised that some tests don't pass due to locale issue whereas they don't configure locale (regress/lib/libc/strerror for example). So I have try to specify "C" locale in test, but it failed: strerror don't relied on locate category, but on LC_* environment. Searching why, I note strerror use catopen which directly manipulate LC_* environment to retrieve the localisation. As this behaviour seems to be not conform to POSIX, and others BSD don't do that, the following patch was proposed :) > [...] > libc's strftime() calls setlocale(). This should be fixed eventually. > I believe it should call some internal thread-safe interface instead, > which we don't have yet. I hope I come back to catopen (and strftime) later. For now running regress with env -i will be enought for me. Thanks for your comments. -- Sébastien Marie
patch: catopen(3), locale setting and getenv
Hi,
I have a question about us catopen(3) implementation: currently we
bypass locale settings (that could be setted from setlocale(3)) to
directly go reading user locale settings from environment.
So a program that try to set specific locale (using setlocale(LC_ALL,
"C") for example), will not use the "C" locale for LC_MESSAGES (when
using catopen).
The current code is:
--- from lib/libc/nls/catopen.c ---
// in _catopen(const char *name, int oflag)
lang = NULL;
if (oflag & NL_CAT_LOCALE) {
lang = getenv("LC_ALL");
if (lang == NULL)
lang = getenv("LC_MESSAGES");
}
if (lang == NULL)
lang = getenv("LANG");
if (lang == NULL)
lang = NLS_DEFAULT_LANG;
if (strcmp(lang, "POSIX") == 0)
lang = NLS_DEFAULT_LANG;
---
The POSIX specification is not very clear, as it speak about
"setting of LC_MESSAGES" and "LANG environment variable".
> This default may be affected by the setting of LC_MESSAGES if the
> value of oflag is NL_CAT_LOCALE, or the LANG environment variable if
> oflag is 0.
Others implementations checked use LC_MESSAGES from locale setting
(instead of value from environment), and fallback to getenv:
- NetBSD
http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libc/nls/catopen.c?rev=1.33&content-type=text/x-cvsweb-markup
if (oflag == NL_CAT_LOCALE) {
lang = loc->part_name[LC_MESSAGES];
}
- FreeBSD
https://svnweb.freebsd.org/base/head/lib/libc/nls/msgcat.c?revision=278530&view=markup
if (type == NL_CAT_LOCALE)
lang = setlocale(LC_MESSAGES, NULL);
else
lang = getenv("LANG");
- DragonFlyBSD (same code as FreeBSD)
http://gitweb.dragonflybsd.org/dragonfly.git/blob/HEAD:/lib/libc/nls/msgcat.c
(same code as FreeBSD)
- Linux
http://sourceware.org/git/?p=glibc.git;a=blob;f=catgets/catgets.c;h=cf93d56c5407ed737cdb8975b77a8b1e5aa26903;hb=HEAD
if (flag == NL_CAT_LOCALE)
/* Use the current locale setting for LC_MESSAGES. */
env_var = setlocale (LC_MESSAGES, NULL);
else
/* Use the LANG environment variable. */
env_var = getenv ("LANG");
The following patch change the behaviour of catopen(3). It will have
impact on program that use catopen(3) (or related functions like
strerror(3)) and don't initialise locale settings using
setlocale(LC_ALL, ""), as the default value for locale (without
initialisation) is "C".
One example in base is kill program:
- don't use setlocale
- use err(3) function (which use strerror, which use catopen)
Generic example:
$ cat catopen-test.c
#include
#include
#include
#include
#include
int main() {
//setlocale(LC_ALL, "");
printf("EPERM = %s\n", strerror(EPERM));
return (EXIT_SUCCESS);
}
Old behaviour:
$ env -i LC_ALL=fr_FR.UTF-8 ./catopen-test
EPERM = Opération non autorisée
New behaviour:
$ env -i LC_ALL=fr_FR.UTF-8 ./catopen-test
EPERM = Operation not permitted
Same program with setlocale initialisation:
$ env -i LC_ALL=fr_FR.UTF-8 ./catopen-test
EPERM = Opération non autorisée
If this change is desirable, I will propose patchs for programs in base
in order to call setlocale(LC_ALL, "") at program initilisation.
Comments, ideas ?
--
Sébastien Marie
Index: nls/catopen.c
===
RCS file: /cvs/src/lib/libc/nls/catopen.c,v
retrieving revision 1.16
diff -u -p -r1.16 catopen.c
--- nls/catopen.c 16 Jan 2015 16:48:51 - 1.16
+++ nls/catopen.c 12 Jun 2015 06:37:22 -
@@ -39,6 +39,7 @@
#include
#include
#include
+#include
#define NLS_DEFAULT_PATH
"/usr/share/nls/%L/%N.cat:/usr/share/nls/%l.%c/%N.cat:/usr/share/nls/%l/%N.cat"
#define NLS_DEFAULT_LANG "C"
@@ -68,9 +69,7 @@ _catopen(const char *name, int oflag)
lang = NULL;
if (oflag & NL_CAT_LOCALE) {
- lang = getenv("LC_ALL");
- if (lang == NULL)
- lang = getenv("LC_MESSAGES");
+ lang = setlocale(LC_MESSAGES, NULL);
}
if (lang == NULL)
lang = getenv("LANG");
[patch] libc/locale/setlocale.c: remove unused arg
Hi,
I start reading libc/locale code in order to understanding it.
Here a patch to remove an unused argument "isspecial" from static
function "load_locale_sub".
The function is called once, with isspecial=0, and the argument isn't
used in function's body.
Any comments ?
--
Sébastien Marie
Index: locale/setlocale.c
===
RCS file: /cvs/src/lib/libc/locale/setlocale.c,v
retrieving revision 1.20
diff -u -p -r1.20 setlocale.c
--- locale/setlocale.c 28 Aug 2013 16:53:34 - 1.20
+++ locale/setlocale.c 9 Jun 2015 17:40:42 -
@@ -77,7 +77,7 @@ static char current_locale_string[_LC_LA
static char*currentlocale(void);
static void revert_to_default(int);
-static int load_locale_sub(int, const char *, int);
+static int load_locale_sub(int, const char *);
static char*loadlocale(int);
static const char *__get_locale_env(int);
@@ -238,7 +238,7 @@ set_lc_messages_locale(const char *locna
}
static int
-load_locale_sub(int category, const char *locname, int isspecial)
+load_locale_sub(int category, const char *locname)
{
/* check for the default locales */
if (!strcmp(new_categories[category], "C") ||
@@ -278,7 +278,7 @@ loadlocale(int category)
current_categories[category]) == 0)
return (current_categories[category]);
- if (!load_locale_sub(category, new_categories[category], 0)) {
+ if (!load_locale_sub(category, new_categories[category])) {
(void)strlcpy(current_categories[category],
new_categories[category],
sizeof(current_categories[category]));
return current_categories[category];
Re: [patch] file(1) determine file type of stdin
On Fri, May 29, 2015 at 12:27:16PM +0100, Nicholas Marriott wrote:
> Here is a slightly tweaked version of your diff.
>
I am not completely ok (see below)
> Index: file.c
> ===
> RCS file: /cvs/src/usr.bin/file/file.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 file.c
> --- file.c29 May 2015 11:03:37 - 1.41
> +++ file.c29 May 2015 11:21:52 -
> @@ -433,7 +439,7 @@ try_stat(struct input_file *inf)
> strerror(inf->msg->error));
> return (1);
> }
> - if (sflag) {
> + if (sflag || strcmp(inf->path, "-") == 0) {
> switch (inf->msg->sb.st_mode & S_IFMT) {
> case S_IFBLK:
> case S_IFCHR:
If we collapse sflag with "-", a pipe will not be "expanded" by default.
$ echo foobar | file -
/dev/stdin: fifo (named pipe)
I just also see a bogus behaviour compared to previous file(1) version:
$ echo foobar | file -s -
/dev/stdin: fifo (named pipe)
I think fifo (S_IFIFO) should be attempted to be read too.
5.5$ mkfifo test
5.5$ (echo 123 > test) &
[1] 18981
5.5$ file -s test
test: ASCII text
Next a slightly modified patch.
--
Sébastien Marie
Index: file.1
===
RCS file: /cvs/src/usr.bin/file/file.1,v
retrieving revision 1.41
diff -u -p -r1.41 file.1
--- file.1 27 Apr 2015 11:12:49 - 1.41
+++ file.1 29 May 2015 19:32:43 -
@@ -74,6 +74,14 @@ or
.Em data
meaning anything else.
.Pp
+If
+.Ar file
+is a single dash
+.Pq Sq -
+,
+.Nm
+reads from the standard input.
+.Pp
The options are as follows:
.Bl -tag -width indent
.It Fl b
@@ -92,7 +100,7 @@ rather than
.It Fl L
Causes symlinks to be followed.
.It Fl s
-Attempts to read block and character device files, not just regular files.
+Attempts to read block, character device and fifo files, not just regular
files.
.It Fl W
Displays warnings when parsing the magic file or applying its tests.
Usually used for debugging.
Index: file.c
===
RCS file: /cvs/src/usr.bin/file/file.c,v
retrieving revision 1.44
diff -u -p -r1.44 file.c
--- file.c 29 May 2015 15:58:34 - 1.44
+++ file.c 29 May 2015 19:32:43 -
@@ -209,7 +209,13 @@ main(int argc, char **argv)
memset(&msg, 0, sizeof msg);
msg.idx = idx;
- if (lstat(argv[idx], &msg.sb) == -1) {
+ if (strcmp(argv[idx], "-") == 0) {
+ if (fstat(STDIN_FILENO, &msg.sb) == -1) {
+ fd = -1;
+ msg.error = errno;
+ } else
+ fd = STDIN_FILENO;
+ } else if (lstat(argv[idx], &msg.sb) == -1) {
fd = -1;
msg.error = errno;
} else {
@@ -441,11 +447,12 @@ try_stat(struct input_file *inf)
strerror(inf->msg->error));
return (1);
}
- if (sflag) {
+ if (sflag || strcmp(inf->path, "-") == 0) {
switch (inf->msg->sb.st_mode & S_IFMT) {
case S_IFBLK:
case S_IFCHR:
case S_IFREG:
+ case S_IFIFO:
return (0);
}
}
@@ -617,7 +624,10 @@ test_file(struct input_file *inf, size_t
if (bflag)
printf("%s\n", inf->result);
else {
- xasprintf(&label, "%s:", inf->path);
+ if (strcmp(inf->path, "-") == 0)
+ xasprintf(&label, "/dev/stdin:");
+ else
+ xasprintf(&label, "%s:", inf->path);
printf("%-*s %s\n", (int)width, label, inf->result);
free(label);
}
Re: [patch] file(1) determine file type of stdin
On Fri, May 29, 2015 at 12:00:36PM +0100, Nicholas Marriott wrote:
> Hi
>
> Why do you want to set O_NONBLOCK at all?
>
> > } else {
> > fd = open(argv[idx], O_RDONLY|O_NONBLOCK);
> > if (fd == -1 && (errno == ENFILE || errno == EMFILE))
Because of open is used with O_NONBLOCK ?
But, if it isn't need it could be removed from my patch.
--
Sébastien Marie
Re: [patch] file(1) determine file type of stdin
On Thu, May 28, 2015 at 11:52:22PM +0100, Nicholas Marriott wrote:
> Hi
>
> Can you make a patch against -current? Your source tree seems pretty out
> of date.
arg, my bads.
> Yes please check the return values from fcntl and fstat.
I added check for fstat, but not for fcntl.
The problem with fcntl is when '-' is "/dev/zero" (for example), fcntl
return error "Operation not supported by device".
$ file - < /dev/zero
-: cannot stat '-' (Operation not supported by device)
There are two problems:
- inconsistency with "file /dev/zero" as it works well
- invalid error message (cannot stat), whereas it is "cannot fcntl"
What do you think ? Should I trait fcntl(O_NONBLOCK) as error ?
I have also integrated the comment from sthen@ in order to have file
working like before:
- mention "/dev/stdin" instead of "-" in report
- don't need to pass -s when "-" is used
I have changed the file.1 patch too.
> > file(1) in 5.5:
> > ---
> >
> > $ echo foobar | file -
> > /dev/stdin: ASCII text
> >
> > $ echo foobar | file -s /dev/stdin
> > /dev/stdin: ASCII text
> >
> >
> >
> > file(1) in -current:
> >
> >
> > $ echo foobar | file -
> > -: cannot stat '-' (No such file or directory)
> >
> > $ echo foobar | file -s /dev/stdin
> > /dev/stdin: data
> >
> >
> >
> > after patching:
> > ---
> >
$ echo foobar | file -
/dev/stdin: ASCII text
$ file - < file.c
/dev/stdin: ASCII C program text
$ file ./- # process the file named '-'
./-: cannot stat './-' (No such file or directory)
Thanks.
--
Sébastien Marie
Index: file.1
===
RCS file: /cvs/src/usr.bin/file/file.1,v
retrieving revision 1.41
diff -u -p -r1.41 file.1
--- file.1 27 Apr 2015 11:12:49 - 1.41
+++ file.1 29 May 2015 10:30:53 -
@@ -74,6 +74,14 @@ or
.Em data
meaning anything else.
.Pp
+If
+.Ar file
+is a single dash
+.Pq Sq -
+,
+.Nm
+reads from the standard input.
+.Pp
The options are as follows:
.Bl -tag -width indent
.It Fl b
Index: file.c
===
RCS file: /cvs/src/usr.bin/file/file.c,v
retrieving revision 1.39
diff -u -p -r1.39 file.c
--- file.c 28 May 2015 19:26:37 - 1.39
+++ file.c 29 May 2015 10:30:53 -
@@ -208,9 +208,20 @@ main(int argc, char **argv)
memset(&msg, 0, sizeof msg);
msg.idx = idx;
- if (lstat(argv[idx], &msg.sb) == -1) {
+ if (strcmp(argv[idx], "-") == 0) {
+ if (fstat(STDIN_FILENO, &msg.sb) == -1) {
+ fd = -1;
+ msg.error = errno;
+
+ } else {
+ fd = STDIN_FILENO;
+ (void)fcntl(fd, O_NONBLOCK);
+ }
+
+ } else if (lstat(argv[idx], &msg.sb) == -1) {
fd = -1;
msg.error = errno;
+
} else {
fd = open(argv[idx], O_RDONLY|O_NONBLOCK);
if (fd == -1 && (errno == ENFILE || errno == EMFILE))
@@ -435,6 +447,10 @@ try_stat(struct input_file *inf)
strerror(inf->msg->error));
return (1);
}
+
+ if (strcmp(inf->path, "-") == 0)
+ return (0);
+
if (sflag) {
switch (inf->msg->sb.st_mode & S_IFMT) {
case S_IFBLK:
@@ -611,7 +627,10 @@ test_file(struct input_file *inf, size_t
if (bflag)
printf("%s\n", inf->result);
else {
- xasprintf(&label, "%s:", inf->path);
+ if (strcmp(inf->path, "-") == 0)
+ xasprintf(&label, "/dev/stdin:");
+ else
+ xasprintf(&label, "%s:", inf->path);
printf("%-*s %s\n", (int)width, label, inf->result);
free(label);
}
[patch] file(1) adjust size in fill_buffer
Hi, While working on file(1) for permit standard input type determination, I investigate why when /dev/stdin is used, the output was 'data'. It seems that the fill_buffer function don't adjust inf->size to readed buffer. So the test_file function may operate on larger buffer than expected, considering garbage as potential data. before: --- $ echo foobar | file -s /dev/stdin /dev/stdin: data after: -- $ echo foobar | file -s /dev/stdin /dev/stdin: ASCII text Thanks. -- Sébastien Marie Index: file.c === RCS file: /cvs/src/usr.bin/file/file.c,v retrieving revision 1.39 diff -u -p -r1.39 file.c --- file.c 28 May 2015 19:26:37 - 1.39 +++ file.c 29 May 2015 10:23:17 - @@ -398,6 +409,7 @@ fill_buffer(struct input_file *inf) left -= got; } + inf->size -= left; return buffer; }
[patch] file(1) determine file type of stdin
Hi,
It was possible, using the previous file(1) version, to determine the
file type of stdin stream. The current version don't permit that (or I
don't found how to do that).
I would propose a patch in order to support '-' argument to be trait as
STDIN_FILENO descriptor.
file(1) in 5.5:
---
$ echo foobar | file -
/dev/stdin: ASCII text
$ echo foobar | file -s /dev/stdin
/dev/stdin: ASCII text
file(1) in -current:
$ echo foobar | file -
-: cannot stat '-' (No such file or directory)
$ echo foobar | file -s /dev/stdin
/dev/stdin: data
after patching:
---
$ echo foobar | file -
-: fifo (named pipe)
$ echo foobar | file -s -
-: ASCII text
$ file - < file.c
-: ASCII C program text
$ file ./- # process the file named '-'
./-: cannot stat './-' (No such file or directory)
$ echo foobar | file -s /dev/stdin # behaviour not changed
/dev/stdin: data
About the patch:
- my english wording is somehow hesitant: it isn't my native language,
but you should already have noted that :). So corrections in manpage
are expected.
- I am not sure about mdoc tags that I used (.Em for "stdin" for
example).
- I am not sure if catching fcntl(3) or fstat(3) errors would make sens
or not.
- Same for configure STDIN_FILENO for O_NONBLOCK.
Remaining interrogations:
- should '-' implies '-s' (as before) ?
- should I rename "-" to "/dev/stdin" in order to have similar output
than before ?
- why "file -s /dev/stdin" returns "data" instead of proper
determination ?
- should I trait "/dev/stdin" as STDIN_FILENO too ?
Thanks.
--
Sébastien Marie
Index: file.1
===
RCS file: /cvs/src/usr.bin/file/file.1,v
retrieving revision 1.39
diff -u -p -r1.39 file.1
--- file.1 24 Apr 2015 20:57:51 - 1.39
+++ file.1 28 May 2015 18:07:15 -
@@ -74,6 +74,16 @@ or
.Em data
meaning anything else.
.Pp
+If the argument is
+.Dq -
+, it is traited as
+.Em stdin
+stream. When
+.Em stdin
+is a pipe, consider use of
+.Fl s
+option in order to determine the type of the stream's content.
+.Pp
The options are as follows:
.Bl -tag -width indent
.It Fl b
Index: file.c
===
RCS file: /cvs/src/usr.bin/file/file.c,v
retrieving revision 1.32
diff -u -p -r1.32 file.c
--- file.c 24 Apr 2015 17:34:57 - 1.32
+++ file.c 28 May 2015 18:07:15 -
@@ -197,6 +197,13 @@ static void
open_file(struct input_file *inf)
{
int retval;
+
+ if (strcmp(inf->path, "-") == 0) {
+ inf->fd = STDIN_FILENO;
+ fcntl(STDIN_FILENO, O_NONBLOCK);
+ fstat(STDIN_FILENO, &inf->sb);
+ return;
+ }
retval = lstat(inf->path, &inf->sb);
if (retval == -1) {
[patch] correct file(1) printed usage
Hi,
I would report (and correct) an invalid usage statement of file(1).
$ file
usage: file [-bchiLsW] [file ...]
$ man file | grep -A1 'SY'
SYNOPSIS
file [-bchiLsW] file ...
As at least one argument is mandatory, removing the [] would make sens.
Thanks.
--
Sébastien Marie
Index: file.c
===
RCS file: /cvs/src/usr.bin/file/file.c,v
retrieving revision 1.32
diff -u -p -r1.32 file.c
--- file.c 24 Apr 2015 17:34:57 - 1.32
+++ file.c 28 May 2015 17:30:09 -
@@ -85,7 +85,7 @@ static struct option longopts[] = {
__dead void
usage(void)
{
- fprintf(stderr, "usage: %s [-bchiLsW] [file ...]\n", __progname);
+ fprintf(stderr, "usage: %s [-bchiLsW] file ...\n", __progname);
exit(1);
}
Re: bug/inconsistency in OpenBSD sed(1) vs. FreeBSD sed(1) [patch]
On Sat, May 09, 2015 at 06:47:05AM +0200, Sébastien Marie wrote: > > Hi, > > Here a small patch to sed to make 'i' and 'a' command to always append > "\n" after 'text'. > > While here, remove 'len' field from 'struct s_appends'. It was just used > for AP_STRING (used for 'a' command), and the switch from fwrite to > printf permit to not use it. > After realizing that even if the patch is simple, it could break a lot of thing (changing the behaviour of 'i' and 'a' command will necessary break some sed scripts), I do a little review of potential breaking candidates in base (and same have to be done in ports). So now, I am not sure that it is a good idea... Next are the files that use (or seems to use) 'i' or 'a' with sed: games/fortune/tools/do_sort gnu/gcc/fixincludes/fixincl.x gnu/gcc/fixincludes/inclhack.def gnu/lib/libstdc++/libstdc++/mkcheck.in gnu/usr.bin/binutils-2.17/intl/linux-msg.sed gnu/usr.bin/binutils-2.17/intl/po2tbl.sed.in gnu/usr.bin/binutils-2.17/intl/xopen-msg.sed gnu/usr.bin/binutils/binutils/mpw-make.sed gnu/usr.bin/binutils/gas/mpw-make.sed gnu/usr.bin/binutils/gdb/config/djgpp/config.sed gnu/usr.bin/binutils/intl/linux-msg.sed gnu/usr.bin/binutils/intl/po2tbl.sed.in gnu/usr.bin/binutils/intl/xopen-msg.sed gnu/usr.bin/gcc/contrib/gcc_update gnu/usr.bin/gcc/gcc/fixinc/fixinc.ptx gnu/usr.bin/gcc/gcc/fixinc/fixinc.svr4 gnu/usr.bin/gcc/gcc/fixinc/fixincl.x gnu/usr.bin/gcc/gcc/fixinc/inclhack.def gnu/usr.bin/texinfo/djgpp/config.sed regress/usr.bin/sed/dc.sed regress/usr.bin/sed/hanoi.sed regress/usr.bin/sed/math.sed regress/usr.bin/sed/sedtest.sh sys/kern/makesyscalls.sh usr.bin/sed/POSIX usr.bin/sed/TEST/hanoi.sed usr.bin/sed/TEST/math.sed usr.bin/sed/TEST/sed.test -- Sébastien Marie
Re: bug/inconsistency in OpenBSD sed(1) vs. FreeBSD sed(1) [patch]
On Fri, May 08, 2015 at 08:27:55PM -0500, Tim Chase wrote:
> On 2015-05-08 14:30, Todd C. Miller wrote:
> > Solaris and Linux also produce "x\ny\nz\n" so that is likely the
> > expected behavior.
>
> I bumped against it while testing some scripts across various
> platforms where I saw Linux & FreeBSD yielding "x\ny\nz\n", while
> OpenBSD was the only anomaly, producing "xy\nz". Thanks for testing
> Solaris.
>
Hi,
Here a small patch to sed to make 'i' and 'a' command to always append
"\n" after 'text'.
While here, remove 'len' field from 'struct s_appends'. It was just used
for AP_STRING (used for 'a' command), and the switch from fwrite to
printf permit to not use it.
--
Sébastien Marie
Index: defs.h
===
RCS file: /cvs/src/usr.bin/sed/defs.h,v
retrieving revision 1.5
diff -u -p -r1.5 defs.h
--- defs.h 19 Jan 2015 15:30:52 - 1.5
+++ defs.h 9 May 2015 04:41:32 -
@@ -113,7 +113,6 @@ enum e_args {
struct s_appends {
enum {AP_STRING, AP_FILE} type;
char *s;
- size_t len;
};
enum e_spflag {
Index: process.c
===
RCS file: /cvs/src/usr.bin/sed/process.c,v
retrieving revision 1.23
diff -u -p -r1.23 process.c
--- process.c 18 Apr 2015 18:28:37 - 1.23
+++ process.c 9 May 2015 04:41:32 -
@@ -109,7 +109,6 @@ redirect:
}
appends[appendx].type = AP_STRING;
appends[appendx].s = cp->t;
- appends[appendx].len = strlen(cp->t);
appendx++;
break;
case 'b':
@@ -151,7 +150,7 @@ redirect:
cspace(&HS, ps, psl, 0);
break;
case 'i':
- (void)printf("%s", cp->t);
+ (void)printf("%s\n", cp->t);
break;
case 'l':
lputs(ps);
@@ -204,7 +203,6 @@ redirect:
}
appends[appendx].type = AP_FILE;
appends[appendx].s = cp->t;
- appends[appendx].len = strlen(cp->t);
appendx++;
break;
case 's':
@@ -424,8 +422,7 @@ flush_appends(void)
for (i = 0; i < appendx; i++)
switch (appends[i].type) {
case AP_STRING:
- fwrite(appends[i].s, sizeof(char), appends[i].len,
- stdout);
+ (void)printf("%s\n", appends[i].s);
break;
case AP_FILE:
/*
file: crash with invalid magic file (+ patch)
Hi,
I would like to report a crash (coredump) with an invalid magic file
(MALLOC_OPTIONS=S is need to expose the bug).
--- ~/.magic ---
0 beshort 0xffd8 JPEG image data
!:mime image/jpeg
>6 string JFIF\
--- end of file ---
The problem is on the last line: the function magic_get_string, used for
get the "JFIF\" string, miss the end-of-line due to '\' char, resulting
processing outside the line variable.
Problem found using afl-fuzz.
The proposed diff ensure '\0' is correctly detected, and return an
error ("can't parse string").
--
Sébastien Marie
Index: magic-load.c
===
RCS file: /cvs/src/usr.bin/file/magic-load.c,v
retrieving revision 1.2
diff -u -p -r1.2 magic-load.c
--- magic-load.c24 Apr 2015 16:45:32 - 1.2
+++ magic-load.c25 Apr 2015 18:21:04 -
@@ -479,6 +479,8 @@ magic_get_string(char **line, char *out,
case '\"':
*out++ = '\"';
break;
+ case '\0':
+ return (-1);
default:
*out++ = c;
break;
PATCH: ksh: parsing problem: quote in comment in command substitution
On Sat, Mar 07, 2015 at 07:16:53AM +0100, Sébastien Marie wrote: > Hi, > > I encounter a problem of parsing in ksh(1): a quote in a comment in a > command substitution $(...) or `...` is parsed as quote and a closing quote > is expected. > > Here code snippet that expose the problem: > > $ cat test.sh > echo $(echo abc | > # comment > cat) > > echo $(echo abc | > # comment with quote ' > cat) > > $ ksh ./test.sh > abc > ./test.sh[9]: no closing quote > > > As side note, bash(1) don't have this parsing problem. > > $ bash ./test.sh > abc > abc > $ > > Thanks. Here a patch to address this problem. It just adds comment stripping in $(...) parsing. I think the patch need good review: I haven't tested it deeply. Thanks. -- Sébastien Marie Index: lex.c === RCS file: /cvs/src/bin/ksh/lex.c,v retrieving revision 1.49 diff -u -p -r1.49 lex.c --- lex.c 17 Dec 2013 16:37:06 - 1.49 +++ lex.c 9 Mar 2015 05:18:19 - @@ -475,6 +475,12 @@ yylex(int cf) statep->ls_scsparen.csstate = 4; ignore_backslash_newline++; break; + case '#': + ignore_backslash_newline++; + while ((c = getsc()) != '\0' && c != '\n') + ; + ignore_backslash_newline--; + ungetsc(c); } break;
Re: [patch] sed: missing bound check resulting stack overflow
On Thu, Dec 11, 2014 at 04:38:50PM +1100, Jonathan Gray wrote:
>
> Yes, I agree. I plan to commit this version:
>
It is ok for me.
Thanks.
Sébastien Marie
> Index: compile.c
> ===
> RCS file: /cvs/src/usr.bin/sed/compile.c,v
> retrieving revision 1.36
> diff -u -p -r1.36 compile.c
> --- compile.c 8 Oct 2014 04:19:08 - 1.36
> +++ compile.c 11 Dec 2014 05:32:42 -
> @@ -538,7 +538,7 @@ compile_flags(char *p, struct s_subst *s
> {
> int gn; /* True if we have seen g or n */
> long l;
> - char wfile[PATH_MAX], *q;
> + char wfile[PATH_MAX], *q, *eq;
>
> s->n = 1; /* Default */
> s->p = 0;
> @@ -584,9 +584,12 @@ compile_flags(char *p, struct s_subst *s
> #endif
> EATSPACE();
> q = wfile;
> + eq = wfile + sizeof(wfile) - 1;
> while (*p) {
> if (*p == '\n')
> break;
> + if (q >= eq)
> + err(COMPILE, "wfile too long");
> *q++ = *p++;
> }
> *q = '\0';
Re: [patch] sed: segfault due to use of initialized variable
On Wed, Dec 10, 2014 at 10:05:49PM +1100, Jonathan Gray wrote: > On Wed, Dec 10, 2014 at 10:39:37AM +0100, Sébastien Marie wrote: > > > > Hi, > > > > Fuzzing sed with afl, I found a crash due to use of uninitialized > > variable. > > > > In process.c oldpsl variable need to be initialized: > > > > $ echo | sed -e 'g;P' > > Segmentation fault (core dumped) > > > > The following patch correct this. > > > > I also include the initialization of p, as it is reported by compiler > > warning too (with -Wall -O2). > > Perhaps the following instead to rework the code to seperate out > the path that needs the pointer swap? > ok, no problem with this version. -- Sébastien Marie
Re: [patch] sed: missing bound check resulting stack overflow
Hi Jonathan,
I think there is a mistake in pointer comparaison (q + 1 >= eq): it
results we keep two chars at end (whereas only one is necessary for
'\0').
- eq points to the last cell in array before out-of-bound.
eq = wfile + sizeof(wfile) - 1;
- q points to the cell that would receive a new character.
The err terminaison should be: when q points to eq (because if we do it,
there is no more place for '\0').
Actually it is "if (q + 1 >= eq)", so we stop when q+1 points to the
last cell, or when q points to the cell before the last cell. As it would
never write to it (we stop): we keep two cells at end.
As simple test, defining wfile to "char wfile[2]", don't permit to save
to any filename, whereas one-char filename should be ok.
$ echo | sed -e s/a//w_
sed: 1: "s/a//w_": wfile too long
I think the test should be "if (q >= eq)".
--
Sébastien Marie
On Wed, Dec 10, 2014 at 10:25:11PM +1100, Jonathan Gray wrote:
> On Wed, Dec 10, 2014 at 11:46:57AM +0100, Sébastien Marie wrote:
> > On Wed, Dec 10, 2014 at 11:16:21AM +0100, Sébastien Marie wrote:
> > > Hi,
> > >
> > > In compile_flags, the variable holding the filename ('w' flag of 's'
> > > command) is an array with PATH_MAX length.
> > >
> > > We should check the size of wanted filename, before copying it in wfile.
> > >
>
> Index: compile.c
> ===
> RCS file: /cvs/src/usr.bin/sed/compile.c,v
> retrieving revision 1.36
> diff -u -p -r1.36 compile.c
> --- compile.c 8 Oct 2014 04:19:08 - 1.36
> +++ compile.c 10 Dec 2014 11:18:14 -
> @@ -538,7 +538,7 @@ compile_flags(char *p, struct s_subst *s
> {
> int gn; /* True if we have seen g or n */
> long l;
> - char wfile[PATH_MAX], *q;
> + char wfile[PATH_MAX], *q, *eq;
>
> s->n = 1; /* Default */
> s->p = 0;
> @@ -584,9 +584,12 @@ compile_flags(char *p, struct s_subst *s
> #endif
> EATSPACE();
> q = wfile;
> + eq = wfile + sizeof(wfile) - 1;
> while (*p) {
> if (*p == '\n')
> break;
> + if (q + 1 >= eq)
> + err(COMPILE, "wfile too long");
> *q++ = *p++;
> }
> *q = '\0';
Re: [patch] sed: missing bound check resulting stack overflow
On Wed, Dec 10, 2014 at 11:16:21AM +0100, Sébastien Marie wrote:
> Hi,
>
> In compile_flags, the variable holding the filename ('w' flag of 's'
> command) is an array with PATH_MAX length.
>
> We should check the size of wanted filename, before copying it in wfile.
>
> $ echo | sed -e s/a//w`perl -e "print '_' x 1"`
> Bus error (core dumped)
>
> Found also with afl-fuzz.
>
Here a new patch that check the file len while copying.
--
Sébastien Marie
Index: compile.c
===
RCS file: /cvs/src/usr.bin/sed/compile.c,v
retrieving revision 1.36
diff -u -p -r1.36 compile.c
--- compile.c 8 Oct 2014 04:19:08 - 1.36
+++ compile.c 10 Dec 2014 10:41:23 -
@@ -587,6 +587,8 @@ compile_flags(char *p, struct s_subst *s
while (*p) {
if (*p == '\n')
break;
+ if (q - wfile + 1 >= sizeof(wfile))
+ err(COMPILE, "wfile too long");
*q++ = *p++;
}
*q = '\0';
Re: [patch] sed: missing bound check resulting stack overflow
On Wed, Dec 10, 2014 at 11:16:21AM +0100, Sébastien Marie wrote:
> Hi,
>
> In compile_flags, the variable holding the filename ('w' flag of 's'
> command) is an array with PATH_MAX length.
>
> We should check the size of wanted filename, before copying it in wfile.
>
> $ echo | sed -e s/a//w`perl -e "print '_' x 1"`
> Bus error (core dumped)
>
> [...]
>
> + if (strnlen(p, PATH_MAX) == PATH_MAX)
> + err(COMPILE, "wfile too long");
My bad, this patch is wrong: p is the whole command (not only the wanted
filename).
I will redo it.
Sorry.
--
Sébastien Marie
[patch] sed: missing bound check resulting stack overflow
Hi,
In compile_flags, the variable holding the filename ('w' flag of 's'
command) is an array with PATH_MAX length.
We should check the size of wanted filename, before copying it in wfile.
$ echo | sed -e s/a//w`perl -e "print '_' x 1"`
Bus error (core dumped)
Found also with afl-fuzz.
Thanks.
--
Sébastien Marie
Index: compile.c
===
RCS file: /cvs/src/usr.bin/sed/compile.c,v
retrieving revision 1.36
diff -u -p -r1.36 compile.c
--- compile.c 8 Oct 2014 04:19:08 - 1.36
+++ compile.c 10 Dec 2014 10:03:51 -
@@ -582,6 +582,8 @@ compile_flags(char *p, struct s_subst *s
return (p);
}
#endif
+ if (strnlen(p, PATH_MAX) == PATH_MAX)
+ err(COMPILE, "wfile too long");
EATSPACE();
q = wfile;
while (*p) {
[patch] sed: segfault due to use of initialized variable
Hi,
Fuzzing sed with afl, I found a crash due to use of uninitialized
variable.
In process.c oldpsl variable need to be initialized:
$ echo | sed -e 'g;P'
Segmentation fault (core dumped)
The following patch correct this.
I also include the initialization of p, as it is reported by compiler
warning too (with -Wall -O2).
Thanks.
--
Sébastien Marie
Index: process.c
===
RCS file: /cvs/src/usr.bin/sed/process.c,v
retrieving revision 1.20
diff -u -p -r1.20 process.c
--- process.c 1 Dec 2014 06:37:25 - 1.20
+++ process.c 10 Dec 2014 09:15:15 -
@@ -83,8 +83,8 @@ process(void)
{
struct s_command *cp;
SPACE tspace;
- size_t len, oldpsl;
- char *p;
+ size_t len, oldpsl = 0;
+ char *p = NULL;
for (linenum = 0; mf_fgets(&PS, REPLACE);) {
Re: patch: correct double-free in dc(1)
On Mon, Nov 24, 2014 at 05:59:20PM +0100, Otto Moerbeek wrote:
> On Mon, Nov 24, 2014 at 01:38:40PM +0100, S??bastien Marie wrote:
>
> > Hi,
> >
> > Starting to play with afl-fuzz, I test it with dc(1), and it found a "Bus
> > error".
> >
>
> Actually, with this, the init in stack_grow can be left out, I believe.
As stack_grow is only used in stack_push{,number,string} and
initialization is done everywhere, I think it is ok.
Yours.
--
Sébastien Marie
patch: correct double-free in dc(1)
Hi, Starting to play with afl-fuzz, I test it with dc(1), and it found a "Bus error". Basically: $ echo '1 2:x1Lx1:x1:x' | dc Bus error (core dumped) I traced the bug, and the code before do a double-free (resulting the Bus error). Thanks to malloc(3) junk :) The problem is a lack of initialisation in stack_pushnumber function. The code before will: - allocate an array on register (call it A) A[1] = 2 - push it on the stack (the array A arrive on stack) - store it in a new array on register (call it B) (the stack is just reduced, the value.array is keeped as it, so address of A is here) B[1] = A - push a number on stack (as the value isn't properly reinitialized, the number is allocated, but the value.array A is keeped) - allocate an array on register - it pops the number (with value.array setted to A), and as it pops, it free the value on stack, and the array A too (as != NULL) - it pops the array B - it will try to set B[1] = 1, so it free B[1], which is A, which is already freed: *boom*. The patch just ensure a push_number (or push_string) properly initialize the value, by set value.array to NULL. -- Sébastien Marie Index: stack.c === RCS file: /cvs/src/usr.bin/dc/stack.c,v retrieving revision 1.11 diff -u -p -r1.11 stack.c --- stack.c 27 Oct 2009 23:59:37 - 1.11 +++ stack.c 24 Nov 2014 12:31:53 - @@ -147,6 +147,7 @@ stack_pushnumber(struct stack *stack, st stack_grow(stack); stack->stack[stack->sp].type = BCODE_NUMBER; stack->stack[stack->sp].u.num = b; + stack->stack[stack->sp].array = NULL; } void @@ -155,6 +156,7 @@ stack_pushstring(struct stack *stack, ch stack_grow(stack); stack->stack[stack->sp].type = BCODE_STRING; stack->stack[stack->sp].u.string = string; + stack->stack[stack->sp].array = NULL; } void
Re: tplink TL-WN722N (ath ar9271): athn0: could not load firmware (and firmware is there)
Hi, On Thu, Sep 25, 2014 at 06:18:11PM -0500, Abel Abraham Camarillo Ojeda wrote: > I have this usb dongle: > > http://www.tp-link.com/en/products/details/?model=tl-wn722n#over > > Tplink TL-WN722N, which according to driver (*.inf) is an atheros ar9271 > device. > > after inserting in usb slot I get: > > athn0 at uhub1 port 3 "ATHEROS USB2.0 WLAN" rev 2.00/1.08 addr 2 > athn0: could not load firmware > > and nothing else happens, with it inserted I ran fw_update(1), and > then tried to remove it and insert it again, still same message. > > should it work? it should. I have the same device, and it works here (-current, but it works also in 5.5). > $ ls -alsh /etc/firmware > 140 -r--r--r-- 1 root bin 69.0K Jul 30 07:04 /etc/firmware/athn-ar7010 > 140 -r--r--r-- 1 root bin 69.0K Jul 30 07:04 /etc/firmware/athn-ar7010-11 > 104 -r--r--r-- 1 root bin 50.1K Jul 30 07:04 /etc/firmware/athn-ar9271 > 8 -r--r--r-- 1 root bin 2.2K Jul 30 07:04 /etc/firmware/athn-license > $ $ ls -alsh /etc/firmware/athn-* 140 -r--r--r-- 1 root bin 69.0K Jan 11 2014 /etc/firmware/athn-ar7010 140 -r--r--r-- 1 root bin 69.0K Jan 11 2014 /etc/firmware/athn-ar7010-11 104 -r--r--r-- 1 root bin 50.1K Jan 11 2014 /etc/firmware/athn-ar9271 8 -r--r--r-- 1 root bin 2.2K Jan 11 2014 /etc/firmware/athn-license $ sha1 /etc/firmware/athn-* SHA1 (/etc/firmware/athn-ar7010) = 4712a8674f6d32b3d2bb548d304177a0fea09558 SHA1 (/etc/firmware/athn-ar7010-11) = 08d0587306e965cd7b04c0715114978395b5ac35 SHA1 (/etc/firmware/athn-ar9271) = 494bc6957da8f6d04d68f8594337fee356c190d3 SHA1 (/etc/firmware/athn-license) = 5fc3ee1bec135933fd8b1c73191e7d7556b9eed0 $ pkg_info athn-firmware Information for inst:athn-firmware-1.1p1 [...] $ dmesg | grep athn athn0 at uhub0 port 4 "ATHEROS USB2.0 WLAN" rev 2.00/1.08 addr 2 athn0: AR9271 rev 1 (1T1R), ROM rev 13, address c0:4a:00:1c:c0:aa -- Sébastien Marie
Re: patch: acpitz: active cooling and notify 0x81
Hi Mike,
On Tue, Sep 16, 2014 at 12:43:11AM -0700, Mike Larkin wrote:
> On Mon, Sep 15, 2014 at 04:57:24PM +0200, S??bastien Marie wrote:
> > ping ?
> >
> > Tihs patch is very conservative: it just allow to switch fan OFF if
> > state is unknown.
>
> I finally read through the relevant parts of the spec. The problem
> here is that it looks like we are implementing thermal zone hysteresis
> incorrectly. I think you noticed this in your original diff, but IMO
> we've gone down a bit of a wrong path here with these later diffs.
>
The first diff try to not go back to "unknown" state for ALx
(sc->sc_ac_stat) when notify 0x81 is received by force reread the ALx
value (but the method used is, at least, unelegant).
The later diffs just copte with unknown state by just ignore it (if we
don't known if fan is on or off, and it should be set to off, so set it
off, even if already in this state). Basically because I suppose that if
sc_ac_stat is set to -1, it should be necessary for some hardware...
The real question would be to known if ALx state should be considered
unknown after hysteresis (if ACx change).
At startup it is ok to considere ALx status to -1 (sometimes, after hot
reboot in particular, the ALx could be ON at startup). But it is not
evident that when notify 0x81 is received, ALx should be considered to
unknown: currently the code set sc->sc_ac_stat[i] to -1 every time.
The following patch try to change this:
- copte with unknown state: permit acpitz_setfan(sc, i, "_OFF") when
sc->sc_ac_stat[i] == -1 (unknown)
- init sc->sc_ac_stat[i] to unknown:
- at system startup (ACPITZ_INIT): the system will "naturally"
initialise it to the good value on first active_cooling.
- on notify 0x82 (ACPITZ_DEVLIST): it seems to me it is
valuable to set unknown state to sc_ac_stat when _ALx change, as
sc_ac_stat reflect the _STA value of _ALx
Thanks.
--
Sébastien Marie
Index: acpitz.c
===
RCS file: /cvs/src/sys/dev/acpi/acpitz.c,v
retrieving revision 1.47
diff -u -p -r1.47 acpitz.c
--- acpitz.c12 Jul 2014 02:44:49 - 1.47
+++ acpitz.c19 Sep 2014 11:39:07 -
@@ -140,7 +142,6 @@ acpitz_init(struct acpitz_softc *sc, int
for (i = 0; i < ACPITZ_MAX_AC; i++) {
snprintf(name, sizeof(name), "_AC%d", i);
sc->sc_ac[i] = acpitz_getreading(sc, name);
- sc->sc_ac_stat[i] = -1;
}
}
@@ -161,6 +162,8 @@ acpitz_init(struct acpitz_softc *sc, int
sc->sc_devnode, &res, 0);
aml_freevalue(&res);
}
+ /* initialize current state to unknown */
+ sc->sc_ac_stat[i] = -1;
}
}
}
@@ -424,7 +439,7 @@ acpitz_refresh(void *arg)
acpitz_setfan(sc, i, "_ON_");
} else if (sc->sc_ac[i] != -1) {
/* turn off fan i */
- if (sc->sc_ac_stat[i] > 0)
+ if ((sc->sc_ac_stat[i] == -1) || (sc->sc_ac_stat[i] >
0))
acpitz_setfan(sc, i, "_OFF");
}
}
Re: patch: acpitz: active cooling and notify 0x81
ping ?
Tihs patch is very conservative: it just allow to switch fan OFF if
state is unknown.
Thanks.
--
Sébastien Marie
On Wed, Aug 27, 2014 at 02:51:20PM +0200, Sébastien Marie wrote:
> Hi Jonathan,
>
> First, thanks for your feedback and for your patch.
>
> On Wed, Aug 27, 2014 at 02:42:43AM +1000, Jonathan Gray wrote:
> > Rather than calling acpi_setfan() again would it make
> > sense to remove the cached state value and move
> > the state reading to just before the method is called?
> >
> > That said I'm not familiar with the acpitz code and
> > haven't gone off to look at the spec.
>
> At first reading I am quite disappointed to remove caching.
>
> So I have:
> - checked in others acpi termal-zone implementation to see if caching
>is using or not for active cooling.
> - try to measure some values on my host.
>
>
> Others implementations checked:
> - netbsd : caching used (sys/dev/acpi/acpi_tz.c, l.368)
> - freebsd : caching used (head/sys/dev/acpica/acpi_thermal.c, l.564)
> - linux : caching seems not be used (but not 100% sure)
>
> (please note I am not expert in theses kernels implementations, so I
> could be wrong).
>
>
> Some measures:
>
> I first try to measure the cost of acpi_setfan in term of time:
> something between 9998432ns et 14574920ns (0.0099 and 0.015 s). It
> seems not being a heavy operation (too my eyes).
>
>
> Secondly, the number of call of acpi_setfan, with and without caching.
>
> The test kernel is build with caching enable. The without-caching
> counter is incremented every time acpi_setfan would be call without
> caching, and with-caching counter incremented only when acpi_setfan is
> called (the patch below was applied: an unknown state would result
> acpi_setfan calling).
>
> During 1h09 :
> - without caching: 3388 calls
> - with caching: 742 calls
>
> The cache is used at 78% of time.
>
>
> So, even if your patch resolve the initial problem (the fan don't keep
> running whereas state is unknown), I would prefer to keep the cache in
> place. But if you think the impact of not use this cache is negligible,
> it is ok for me.
>
> I join a very conservative patch which just allow calling
> acpi_setfan(sc,i,"_OFF") if cache is unknown.
>
> Thanks.
> --
> Sébastien Marie
>
>
> Index: dev/acpi/acpitz.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpitz.c,v
> retrieving revision 1.47
> diff -u -p -r1.47 acpitz.c
> --- dev/acpi/acpitz.c 12 Jul 2014 02:44:49 - 1.47
> +++ dev/acpi/acpitz.c 27 Aug 2014 12:38:36 -
> @@ -424,7 +424,7 @@ acpitz_refresh(void *arg)
> acpitz_setfan(sc, i, "_ON_");
> } else if (sc->sc_ac[i] != -1) {
> /* turn off fan i */
> - if (sc->sc_ac_stat[i] > 0)
> + if (sc->sc_ac_stat[i] == -1 || sc->sc_ac_stat[i] > 0)
> acpitz_setfan(sc, i, "_OFF");
> }
> }
Re: patch: acpitz: active cooling and notify 0x81
Hi Jonathan,
First, thanks for your feedback and for your patch.
On Wed, Aug 27, 2014 at 02:42:43AM +1000, Jonathan Gray wrote:
> Rather than calling acpi_setfan() again would it make
> sense to remove the cached state value and move
> the state reading to just before the method is called?
>
> That said I'm not familiar with the acpitz code and
> haven't gone off to look at the spec.
At first reading I am quite disappointed to remove caching.
So I have:
- checked in others acpi termal-zone implementation to see if caching
is using or not for active cooling.
- try to measure some values on my host.
Others implementations checked:
- netbsd : caching used (sys/dev/acpi/acpi_tz.c, l.368)
- freebsd : caching used (head/sys/dev/acpica/acpi_thermal.c, l.564)
- linux : caching seems not be used (but not 100% sure)
(please note I am not expert in theses kernels implementations, so I
could be wrong).
Some measures:
I first try to measure the cost of acpi_setfan in term of time:
something between 9998432ns et 14574920ns (0.0099 and 0.015 s). It
seems not being a heavy operation (too my eyes).
Secondly, the number of call of acpi_setfan, with and without caching.
The test kernel is build with caching enable. The without-caching
counter is incremented every time acpi_setfan would be call without
caching, and with-caching counter incremented only when acpi_setfan is
called (the patch below was applied: an unknown state would result
acpi_setfan calling).
During 1h09 :
- without caching: 3388 calls
- with caching: 742 calls
The cache is used at 78% of time.
So, even if your patch resolve the initial problem (the fan don't keep
running whereas state is unknown), I would prefer to keep the cache in
place. But if you think the impact of not use this cache is negligible,
it is ok for me.
I join a very conservative patch which just allow calling
acpi_setfan(sc,i,"_OFF") if cache is unknown.
Thanks.
--
Sébastien Marie
Index: dev/acpi/acpitz.c
===
RCS file: /cvs/src/sys/dev/acpi/acpitz.c,v
retrieving revision 1.47
diff -u -p -r1.47 acpitz.c
--- dev/acpi/acpitz.c 12 Jul 2014 02:44:49 - 1.47
+++ dev/acpi/acpitz.c 27 Aug 2014 12:38:36 -
@@ -424,7 +424,7 @@ acpitz_refresh(void *arg)
acpitz_setfan(sc, i, "_ON_");
} else if (sc->sc_ac[i] != -1) {
/* turn off fan i */
- if (sc->sc_ac_stat[i] > 0)
+ if (sc->sc_ac_stat[i] == -1 || sc->sc_ac_stat[i] > 0)
acpitz_setfan(sc, i, "_OFF");
}
}
Re: patch: acpitz: active cooling and notify 0x81 (patch v2)
On Thu, Aug 21, 2014 at 10:44:36AM +0200, Sébastien Marie wrote:
> Hi,
>
> Another possibility (not tested) should be to change active cooling code
> to permit call acpitz_setfan(OFF) when sc_ac_stat == -1.
>
Next is the patch that implement the other possibility (the code is
running, currently fan switchs ON and OFF following the temperature...)
The decision of call acpitz_setfan is now based on explicit values for
power status (as the ACPI spec define only two values: 0=OFF and 1=ON).
It should be valid if ACPI implementations don't use others values for
_STA... but I don't known (and previous code check >0 for ON, not just
==1).
The logic in pseudo code:
if active-point-temperature is valid (!= -1)
if active-point-temperature <= current-temperature (temperature over
active-point: active cooling need)
if power-status != ON
setfan(ON)
else
if power-status != OFF
setfan(OFF)
--
Sébastien Marie
Index: src-sys-current/dev/acpi/acpitz.c
===
--- src-sys-current.orig/dev/acpi/acpitz.c
+++ src-sys-current/dev/acpi/acpitz.c
@@ -417,17 +417,20 @@ acpitz_refresh(void *arg)
sc->sc_lasttmp = sc->sc_tmp;
/* active cooling */
- for (i = 0; i < ACPITZ_MAX_AC; i++) {
- if (sc->sc_ac[i] != -1 && sc->sc_ac[i] <= sc->sc_tmp) {
- /* turn on fan i */
- if (sc->sc_ac_stat[i] <= 0)
- acpitz_setfan(sc, i, "_ON_");
- } else if (sc->sc_ac[i] != -1) {
- /* turn off fan i */
- if (sc->sc_ac_stat[i] > 0)
- acpitz_setfan(sc, i, "_OFF");
+ for (i = 0; i < ACPITZ_MAX_AC; i++)
+ if (sc->sc_ac[i] != -1 ) {
+ if (sc->sc_ac[i] <= sc->sc_tmp) {
+ if (sc->sc_ac_stat[i] != 1)
+ /* turn on fan i */
+ acpitz_setfan(sc, i, "_ON_");
+
+ } else {
+ if (sc->sc_ac_stat[i] != 0)
+ /* turn off fan i */
+ acpitz_setfan(sc, i, "_OFF");
+ }
}
- }
+
sc->sc_sens.value = sc->sc_tmp * 10 - 5;
}
patch: acpitz: active cooling and notify 0x81
Hi,
I have encounter problem with acpitz: basically with acpitz enable,
a fan keep running at max speed (with lot of noise).
I have traced the problem (see
http://marc.info/?l=openbsd-bugs&m=140517557620686&w=2 for reference).
As I haven't received any indications, I go to the ACPI spec in order to
understand what happens. Currently, I'm not sure to understand all the
terms, so please correct my if I am wrong: that would help me !
The host is an HP compaq nc6400 (see dmesg from previous reference if need).
>From what I understand, the host use notify 0x81 when temperature pass
an active point. The spec say it is ok: "The following are the primary
uses for this type of thermal notification:
- ...
- ...
- After the crossing of an active or passive trip point is signaled to
implement hysteresis."
The current code in acpitz.c for 0x81 notify is to reread TRIP points
(passive: _PSV and actives: _ACx), and for active points, to reinitiate
sc_ac_stat (which seems to keep the current state of fan:
-1=uninitialized / 0=OFF / 1=ON).
sc_ac_stat is taken from _STA: "Returns the current ON or OFF status for
the power resource." (0=OFF / 1=ON)
The problem is the code set sc_ac_stat to -1, whereas fan could be ON.
>From dev/acpi/acpitz.c:
>/* active cooling */
>for (i = 0; i < ACPITZ_MAX_AC; i++) {
>if (sc->sc_ac[i] != -1 && sc->sc_ac[i] <= sc->sc_tmp) {
>/* turn on fan i */
>if (sc->sc_ac_stat[i] <= 0)
>acpitz_setfan(sc, i, "_ON_");
>} else if (sc->sc_ac[i] != -1) {
>/* turn off fan i */
>if (sc->sc_ac_stat[i] > 0)
>acpitz_setfan(sc, i, "_OFF");
>}
>}
The active cooling code could set the fan ON if sc_ac_stat is -1, but
couldn't set the fan OFF...
So when a notify 0x81 occurs and the fan is ON, sc_ac_stat is set to -1,
and as the temperature will keep low, the active cooling will not do
anything... keeping the fan running.
So I have change the round-trip reinitialization to not bindly set -1 to
sc_ac_stat, but reread the corresponding _STA instead of.
The following patch acheive this by:
- change acpitz_setfan function to allow a NULL method (instead of
"_ON_" or "_OFF" values only).
The purpose is to reuse the codepath that read _STA, but without set
the fan ON or OFF.
- call acpitz_setfan with NULL during acpitz_init, when reading trip
points. It always set sc_ac_stat=-1, but if no error in acpitz_setfan
occurs, the call will set sc_ac_stat= _STA.
Another possibility (not tested) should be to change active cooling code
to permit call acpitz_setfan(OFF) when sc_ac_stat == -1.
Thanks to comment.
--
Sébastien Marie
Index: src-sys-current/dev/acpi/acpitz.c
===
--- src-sys-current.orig/dev/acpi/acpitz.c
+++ src-sys-current/dev/acpi/acpitz.c
@@ -141,6 +141,7 @@ acpitz_init(struct acpitz_softc *sc, int
snprintf(name, sizeof(name), "_AC%d", i);
sc->sc_ac[i] = acpitz_getreading(sc, name);
sc->sc_ac_stat[i] = -1;
+ acpitz_setfan(sc, i, NULL);
}
}
@@ -317,8 +318,8 @@ acpitz_setfan(struct acpitz_softc *sc, i
DEVNAME(sc), name, x, y);
continue;
}
- if (aml_evalname(sc->sc_acpi, ref->node, method, 0,
- NULL, NULL))
+ if (method != NULL && aml_evalname(sc->sc_acpi,
ref->node,
+ method, 0, NULL, NULL))
printf("%s: %s[%d.%d] %s fails\n",
DEVNAME(sc), name, x, y, method);
man.cgi: clean exit when absent or empty manpath.conf
Hi,
Starting to play with man.cgi (src/usr.bin/mandoc/cgi.c), it seems that
man.cgi will segfault if configuration file is absent or empty.
Here a patch that display error message and 505, like when MAN_DIR is
invalid.
Note: the segfault occurs in cgi.c:224 (http_parse), that assume
req->p is not NULL (req->q.manpath = req->p[0]).
The diff use the same style that when MAN_DIR is invalid (cgi.c:917), but that
could
be improved using err(3) ?
Thanks.
--
Sébastien Marie
Index: cgi.c
===
RCS file: /cvs/src/usr.bin/mandoc/cgi.c,v
retrieving revision 1.13
diff -u -p -r1.13 cgi.c
--- cgi.c 13 Jul 2014 15:38:06 - 1.13
+++ cgi.c 18 Jul 2014 06:43:14 -
@@ -966,8 +968,12 @@ pathgen(struct req *req)
char*dp;
size_t dpsz;
- if (NULL == (fp = fopen("manpath.conf", "r")))
- return;
+ if (NULL == (fp = fopen("manpath.conf", "r"))) {
+ fprintf(stderr, "manpath.conf not found in MAN_DIR (%s)\n",
+ MAN_DIR);
+ pg_error_internal();
+ exit(EXIT_FAILURE);
+ }
while (NULL != (dp = fgetln(fp, &dpsz))) {
if ('\n' == dp[dpsz - 1])
@@ -975,5 +981,11 @@ pathgen(struct req *req)
req->p = mandoc_realloc(req->p,
(req->psz + 1) * sizeof(char *));
req->p[req->psz++] = mandoc_strndup(dp, dpsz);
+ }
+
+ if ( req->p == NULL ) {
+ fprintf(stderr, "empty manpath.conf\n");
+ pg_error_internal();
+ exit(EXIT_FAILURE);
}
}
Re: PATCH: ftp: allow @ in username for Basic Auth
On Sun, Jun 29, 2014 at 09:52:28PM -0700, Philip Guenther wrote: > > > Here's a patch to do that. > > > > Just a comment in code (unused variables in urldecode). > > Else it seems ok. And my use-case works. > > Dang it, I just noticed that I had sent an earlier version of my diff, > which had problems with some proxy setting, IIRC. Here's the diff that I > settled on after testing. > It works also for my use-case. Please note I haven't tested proxy setting (by lake of server to test). Thanks for your help. -- Sébastien Marie
Re: PATCH: ftp: allow @ in username for Basic Auth
On Wed, Jun 25, 2014 at 07:07:30PM -0700, Philip Guenther wrote:
> On Wed, 25 Jun 2014, S?bastien Marie wrote:
> > On Tue, Jun 24, 2014 at 10:55:44AM -0700, Philip Guenther wrote:
> > > On Tue, Jun 24, 2014 at 9:01 AM, S?bastien Marie <
> > > semarie-open...@latrappe.fr> wrote:
> ...
> > So, I think ftp(1) should urldecode userinfo before base64.
>
> I agree.
>
> > I propose another patch that implement urldecoding of userinfo (as it
> > come from URI).
>
> It also needs to be fixed for proxy authentication. I think the code to
> urldecode and reencode in base64 should be factored out.
If urldecode is refactored, it may return the length of decoded string
(as size_t* parameter: NULL for no result, output variable else).
The purpose is to properly managed the case where %00 is include in the
userinfo. Else it results truncation (as string are \0 terminate).
But it would need more work: if in recode_credentials there is no
problem (just need to pass ulen to urldecode, and don't need strlen),
urldecode is used also for FTP URL parsing.
But note that I don't sure that manage %00 in userinfo make sense.
> Here's a patch to do that.
Just a comment in code (unused variables in urldecode).
Else it seems ok. And my use-case works.
> ...
>
> Index: fetch.c
> ===
> RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
> retrieving revision 1.122
> diff -u -p -r1.122 fetch.c
> --- fetch.c 20 May 2014 01:25:23 - 1.122
> +++ fetch.c 26 Jun 2014 01:40:28 -
> @@ -75,7 +75,8 @@ static int url_get(const char *, const c
> void aborthttp(int);
> void abortfile(int);
> char hextochar(const char *);
> -char *urldecode(const char *);
> +void urldecode(char *);
> +char *recode_credentials(char *_userinfo);
> int ftp_printf(FILE *, SSL *, const char *, ...)
> __attribute__((format(printf, 3, 4)));
> char *ftp_readline(FILE *, SSL *, size_t *);
> size_t ftp_read(FILE *, SSL *, char *, size_t);
> @@ -97,8 +98,6 @@ SSL_CTX *ssl_get_ssl_ctx(void);
> #define FTP_PROXY"ftp_proxy" /* env var with ftp proxy location */
> #define HTTP_PROXY "http_proxy"/* env var with http proxy location */
>
> -#define COOKIE_MAX_LEN 42
> -
> #define EMPTYSTRING(x) ((x) == NULL || (*(x) == '\0'))
>
> static const char at_encoding_warning[] =
> @@ -393,7 +392,7 @@ url_get(const char *origline, const char
> struct addrinfo hints, *res0, *res, *ares = NULL;
> const char * volatile savefile;
> char * volatile proxyurl = NULL;
> - char *cookie = NULL;
> + char *credentials = NULL;
> volatile int s = -1, out;
> volatile sig_t oldintr, oldinti;
> FILE *fin = NULL;
> @@ -402,9 +401,9 @@ url_get(const char *origline, const char
> ssize_t len, wlen;
> #ifndef SMALL
> char *sslpath = NULL, *sslhost = NULL;
> - char *locbase, *full_host = NULL, *auth = NULL;
> + char *locbase, *full_host = NULL;
> const char *scheme;
> - int ishttpsurl = 0;
> + int ishttpurl = 0, ishttpsurl = 0;
> SSL_CTX *ssl_ctx = NULL;
> #endif /* !SMALL */
> SSL *ssl = NULL;
> @@ -420,6 +419,7 @@ url_get(const char *origline, const char
> if (strncasecmp(newline, HTTP_URL, sizeof(HTTP_URL) - 1) == 0) {
> host = newline + sizeof(HTTP_URL) - 1;
> #ifndef SMALL
> + ishttpurl = 1;
> scheme = HTTP_URL;
> #endif /* !SMALL */
> } else if (strncasecmp(newline, FTP_URL, sizeof(FTP_URL) - 1) == 0) {
> @@ -472,17 +472,10 @@ noslash:
>* contain the path. Basic auth from RFC 2617, valid
>* characters for path are in RFC 3986 section 3.3.
>*/
> - if (proxyenv == NULL &&
> - (!strcmp(scheme, HTTP_URL) || !strcmp(scheme, HTTPS_URL))) {
> + if (proxyenv == NULL && (ishttpurl || ishttpsurl)) {
> if ((p = strchr(host, '@')) != NULL) {
> - size_t authlen = (strlen(host) + 5) * 4 / 3;
> - *p = 0; /* Kill @ */
> - if ((auth = malloc(authlen)) == NULL)
> - err(1, "Can't allocate memory for "
> - "authorization");
> - if (b64_ntop(host, strlen(host),
> - auth, authlen) == -1)
> - errx(1, "error in base64 encoding");
> + *p = '\0';
> + credentials = recode_credentials(host);
> host = p + 1;
> }
> }
> @@ -544,27 +537,14 @@ noslash:
> path = strchr(host, '@'); /* look for credentials in
> proxy */
> if (!EMPTYSTRING(path)) {
> *path = '\0';
> - cookie = strchr(host, ':');
> - if (EMPTYSTRING(cookie)) {
> + if (strchr(host, ':') == N
Re: PATCH: ftp: allow @ in username for Basic Auth
On Tue, Jun 24, 2014 at 10:55:44AM -0700, Philip Guenther wrote:
> On Tue, Jun 24, 2014 at 9:01 AM, Sébastien Marie <
> semarie-open...@latrappe.fr> wrote:
>
> > As I see not activity or feedback for this one line patch, I think it
> > need more explain ?
> >
>
> Sorry, the patch is incorrect; per RFC 3986, the parser is correct to split
> the authority on the first '@'. The correct method to include an '@' in
> the userinfo part is to percent-encode it as %40.
>
Thanks for pointing me the good RFC for the format of userinfo in URI.
But, I am not sure to understand *where* is located my problem: if it is
a problem in ftp(1), or a problem in specific service I try to used with
ftp (dyndns server, the http reply come from ngnix server).
Currently ftp(1) manage http, https with url_get function. The basic
auth implemented here do *not* try to decode any urlencoded information.
So pass "user%40example.com:password" as userinfo in URI will result to
base64 "user%40example.com:password" (and not
"u...@example.com:password").
The dyndns service reply with "401 Unauthorized".
Comparing with curl(1) (in ports), a user:password information passed as
argument is passed to base64 "as it", but a user:password passed in the
URI is firstly urldecoded before base64.
$ curl -v -u 'n...@example.com:password' 'http://localhost/'
Authorization: Basic bmFtZUBleGFtcGxlLmNvbTpwYXNzd29yZA==
$ curl -v 'http://name%40example.com:password@localhost/'
Authorization: Basic bmFtZUBleGFtcGxlLmNvbTpwYXNzd29yZA==
"bmFtZUBleGFtcGxlLmNvbTpwYXNzd29yZA==" is "n...@example.com:password"
So, I think ftp(1) should urldecode userinfo before base64. I propose
another patch that implement urldecoding of userinfo (as it come from
URI).
Some comments about the patch:
- I stop use p variable for userinfo parse: I declare and use two new
variables, userinfo and userinfoend. I hope the code is more
readable.
- for urldecoding, I use an already defined function. Memory is
allocated and checked by this function.
- I didn't try to parse userinfo as "user:password". The userinfo
string is passed "as it" to urldecode.
Thanks.
--
Sébastien Marie
Index: fetch.c
===
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.122
diff -u -p -r1.122 fetch.c
--- fetch.c 20 May 2014 01:25:23 - 1.122
+++ fetch.c 25 Jun 2014 06:49:11 -
@@ -474,16 +474,30 @@ noslash:
*/
if (proxyenv == NULL &&
(!strcmp(scheme, HTTP_URL) || !strcmp(scheme, HTTPS_URL))) {
- if ((p = strchr(host, '@')) != NULL) {
- size_t authlen = (strlen(host) + 5) * 4 / 3;
- *p = 0; /* Kill @ */
+ char *userinfo, *userinfoend;
+
+ /* extract userinfo part */
+ if ((userinfoend = strchr(host, '@')) != NULL) {
+ size_t authlen;
+
+ /* separate userinfo and host components */
+ userinfo = host;
+ host = userinfoend + 1;
+ *userinfoend = '\0';
+
+ /* urldecode userinfo */
+ userinfo = urldecode(userinfo);
+
+ /* build Basic auth */
+ authlen = (strlen(userinfo) + 5) * 4 / 3;
if ((auth = malloc(authlen)) == NULL)
err(1, "Can't allocate memory for "
"authorization");
- if (b64_ntop(host, strlen(host),
+ if (b64_ntop(userinfo, strlen(userinfo),
auth, authlen) == -1)
errx(1, "error in base64 encoding");
- host = p + 1;
+
+ free(userinfo);
}
}
#endif /* SMALL */
Re: PATCH: ftp: allow @ in username for Basic Auth
Hi,
As I see not activity or feedback for this one line patch, I think it
need more explain ?
Currently, when you pass an URL with user/pass embed, the code parse it
badly.
For example:
https://mym...@example.com:my-passw...@another-domain.example.com/blabla
Just before the code search if the supplied URL contains a user/pass,
the variables are:
scheme = "https://";
host = "mym...@example.com:my-passw...@another-domain.example.com"
The code use strchr(3) on host in order to find '@' in host variable,
and separate the user/pass component and the host component.
But, with strchr the result is:
p = "mymail"
host = "example.com:my-passw...@another-domain.example.com"
The patch replace strchr(3) by strrchr(3) to obtain:
p = "mym...@example.com:my-password"
host = "another-domain.example.com"
As the hostname should not contains '@' char, and user/pass may contains
it, (as defined by rfc1738), this patch make ftp(1) to more respect
standard.
Thanks.
--
Sébastien Marie
On Mon, Jun 23, 2014 at 10:15:25AM +0200, Sébastien Marie wrote:
> Hi,
>
> Using ftp(1) with HTTP(S) scheme and Basic auth, it is currently not
> possible to have username (or password) with a '@' inner.
>
> For example, this URI is badly parsed:
> ftp https://mym...@example.com:my-passw...@another-domain.example.com/blabla
>
> According to RFC2617, '@' is a valid character in user-id or password:
> user-pass = userid ":" password
> userid = *
> password= *TEXT
>
> Here a patch to search the last '@' in the string (which don't contains
> the path at this time).
>
> --
> Sébastien Marie
>
> Index: fetch.c
> ===
> RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
> retrieving revision 1.122
> diff -u -p -r1.122 fetch.c
> --- fetch.c 20 May 2014 01:25:23 - 1.122
> +++ fetch.c 23 Jun 2014 07:46:33 -
> @@ -474,7 +474,7 @@ noslash:
>*/
> if (proxyenv == NULL &&
> (!strcmp(scheme, HTTP_URL) || !strcmp(scheme, HTTPS_URL))) {
> - if ((p = strchr(host, '@')) != NULL) {
> + if ((p = strrchr(host, '@')) != NULL) {
> size_t authlen = (strlen(host) + 5) * 4 / 3;
> *p = 0; /* Kill @ */
> if ((auth = malloc(authlen)) == NULL)
>
PATCH: ftp: allow @ in username for Basic Auth
Hi,
Using ftp(1) with HTTP(S) scheme and Basic auth, it is currently not
possible to have username (or password) with a '@' inner.
For example, this URI is badly parsed:
ftp https://mym...@example.com:my-passw...@another-domain.example.com/blabla
According to RFC2617, '@' is a valid character in user-id or password:
user-pass = userid ":" password
userid = *
password= *TEXT
Here a patch to search the last '@' in the string (which don't contains
the path at this time).
--
Sébastien Marie
Index: fetch.c
===
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.122
diff -u -p -r1.122 fetch.c
--- fetch.c 20 May 2014 01:25:23 - 1.122
+++ fetch.c 23 Jun 2014 07:46:33 -
@@ -474,7 +474,7 @@ noslash:
*/
if (proxyenv == NULL &&
(!strcmp(scheme, HTTP_URL) || !strcmp(scheme, HTTPS_URL))) {
- if ((p = strchr(host, '@')) != NULL) {
+ if ((p = strrchr(host, '@')) != NULL) {
size_t authlen = (strlen(host) + 5) * 4 / 3;
*p = 0; /* Kill @ */
if ((auth = malloc(authlen)) == NULL)
Re: pkg_add (pkg.conf): option to require signed packages
On Thu, Jan 16, 2014 at 10:02:22AM +, Stuart Henderson wrote: > On 2014/01/16 08:53, Sébastien Marie wrote: > > Hi, > > > > Does it make sens to have an option to require package to be signed ? > > It makes more sense to just enable that by default, when we are happy > with the infrastructure and usage. > I saw "enable by default" more as long term purpose. The patch would permit to easily test it... But I am confident about your choices. Thanks. -- Sébastien Marie
pkg_add (pkg.conf): option to require signed packages
Hi,
Does it make sens to have an option to require package to be signed ?
Currently, a package without signature is gracefully installed without
warning.
The patch introduce an option "require-signature" in pkg.conf, and it
respects -Dnosig in comand-line, if present.
Thanks.
--
Sébastien Marie
Index: pkg.conf.5
===
RCS file: /cvs/src/usr.sbin/pkg_add/pkg.conf.5,v
retrieving revision 1.5
diff -u -p -r1.5 pkg.conf.5
--- pkg.conf.5 11 Oct 2012 17:35:45 - 1.5
+++ pkg.conf.5 16 Jan 2014 07:47:30 -
@@ -78,6 +78,10 @@ to waive checksums during package deleti
Set to
.Ar yes
to display (done/total) number of package messages.
+.It Ar require-signature
+Set to
+.Ar yes
+to require packages to be signed.
.El
.Pp
Each option uses a separate line, and follows the following template:
Index: OpenBSD/PkgAdd.pm
===
RCS file: /cvs/src/usr.sbin/pkg_add/OpenBSD/PkgAdd.pm,v
retrieving revision 1.45
diff -u -p -r1.45 PkgAdd.pm
--- OpenBSD/PkgAdd.pm 11 Jan 2014 11:54:43 - 1.45
+++ OpenBSD/PkgAdd.pm 16 Jan 2014 07:47:30 -
@@ -663,6 +663,9 @@ sub check_digital_signature
$state->{check_digest} = 1;
$state->{packages_with_sig}++;
}
+ } elsif ($state->config->istrue("require-signature") and !
$state->defines('nosig')) {
+ $state->fatal("#1 isn't signed and signature is
required",
+ $plist->pkgname);
} else {
$state->{packages_without_sig}{$plist->pkgname} = 1;
}
Remove unused variable in arch/i386/i386/lapic.c
Hi I have noted that "scaled_pentium_mhz" variable in i386/lapic.c is defined and initialized, but never used anywhere (grep -R scaled_pentium_mhz /usr/src say nothing [after patch applied]). Remove it ? -- Sébastien Marie Index: arch/i386/i386/lapic.c === RCS file: /cvs/src/sys/arch/i386/i386/lapic.c,v retrieving revision 1.32 diff -u -p -r1.32 lapic.c --- arch/i386/i386/lapic.c 2 Jun 2013 18:16:42 - 1.32 +++ arch/i386/i386/lapic.c 26 Oct 2013 14:35:03 - @@ -234,7 +234,6 @@ u_int32_t lapic_per_second; u_int32_t lapic_frac_usec_per_cycle; u_int64_t lapic_frac_cycle_per_usec; u_int32_t lapic_delaytab[26]; -u_int64_t scaled_pentium_mhz; void lapic_clockintr(void *arg) @@ -368,8 +367,6 @@ lapic_calibrate_timer(struct cpu_info *c tmp = (lapic_per_second * (u_int64_t)1 << 32) / 100; lapic_frac_cycle_per_usec = tmp; - - scaled_pentium_mhz = (1ULL << 32) / cpuspeed; /* * Compute delay in cycles for likely short delays in usec.
