fuse_parse_cmd_line.3 patch
remove a bunch of `_' where there shouldn't be `_'s Index: fuse_parse_cmd_line.3 === RCS file: /cvs/src/lib/libfuse/fuse_parse_cmd_line.3,v retrieving revision 1.2 diff -u -p -u -r1.2 fuse_parse_cmd_line.3 --- fuse_parse_cmd_line.3 8 Jul 2018 06:17:10 - 1.2 +++ fuse_parse_cmd_line.3 28 Nov 2018 02:54:13 - @@ -15,25 +15,25 @@ .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. .\" .Dd $Mdocdate: July 8 2018 $ -.Dt FUSE_PARSE_CMD_LINE 3 +.Dt FUSE_PARSE_CMDLINE 3 .Os .Sh NAME -.Nm fuse_parse_cmd_line +.Nm fuse_parse_cmdline .Nd FUSE helper function to parse command line arguments .Sh SYNOPSIS .In fuse.h .Ft int -.Fn fuse_parse_cmd_line "struct fuse_args *args" "char **mp" \ +.Fn fuse_parse_cmdline "struct fuse_args *args" "char **mp" \ "int *mt" "int *fg" .Sh DESCRIPTION -.Fn fuse_parse_cmd_line +.Fn fuse_parse_cmdline is a helper function to parse standard FUSE arguments. .Fa args can be constructed using the .Xr FUSE_ARGS_INIT 3 macro. .Pp -.Fn fuse_parse_cmd_line +.Fn fuse_parse_cmdline supports the following arguments. .Bl -tag -width Ds .It Fl d , Fl odebug @@ -65,13 +65,13 @@ Print the FUSE library version to stderr .El .Pp If the first argument not recognised by -.Fn fuse_parse_cmd_line +.Fn fuse_parse_cmdline is a valid directory then .Fa mp will be set to the canonicalized absolute pathname of this directory. .Sh RETURN VALUES The -.Fn fuse_parse_cmd_line +.Fn fuse_parse_cmdline function will return 0 on success and -1 if .Fl h , Fl -help , Fl ho , Fl v or @@ -88,11 +88,11 @@ does not exist or is not a directory. .Xr fuse_setup 3 .Sh STANDARDS The -.Fn fuse_parse_cmd_line +.Fn fuse_parse_cmdline function conforms to FUSE 2.6. .Sh HISTORY The -.Fn fuse_parse_cmd_line +.Fn fuse_parse_cmdline function first appeared in .Ox 5.4 . .Sh AUTHORS
Re: option kcov + GENERIC.MP -> silent crash
I booted the patched kernel and it seems to have gone farther and I believe reached init before crashing. boot> b bsd.anton booting hd0a:bsd.anton: 12380226+2360336+270368+0+675840 [684182+128+754752+529898]=0x10d8f48 entry point at 0x1001000 [ using 1969992 bytes of bsd ELF symbol table ] Copyright (c) 1982, 1986, 1989, 1991, 1993 The Regents of the University of California. All rights reserved. Copyright (c) 1995-2018 OpenBSD. All rights reserved. https://www.OpenBSD.org OpenBSD 6.4-current (SYZKALLER) #0: Tue Nov 27 17:40:55 PST 2018 syzkaller@ci-openbsd.syzkaller :/home/syzkaller/src/sys/arch/amd64/compile/SYZKALLER real mem = 17163079680 (16367MB) avail mem = 16632164352 (15861MB) mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.4 @ 0xbcf0 (20 entries) bios0: vendor Google version "Google" date 01/01/2011 bios0: Google Google Compute Engine acpi0 at bios0: rev 0 acpi0: sleep states S3 S4 S5 acpi0: tables DSDT FACP SSDT APIC WAET SRAT acpi0: wakeup devices acpitimer0 at acpi0: 3579545 Hz, 24 bits acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: Intel(R) Xeon(R) CPU @ 2.30GHz, 2300.42 MHz, 06-3f-00 cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,HTT,SSE3,PCLMUL,VMX,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4. 2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,ARAT,XSAVEOPT,MELTDOWN cpu0: 256KB 64b/line 8-way L2 cache cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges cpu0: apic clock running at 999MHz cpu1 at mainbus0: apid 2 (application processor) cpu1: Intel(R) Xeon(R) CPU @ 2.30GHz, 2299.55 MHz, 06-3f-00 cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,HTT,SSE3,PCLMUL,VMX,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4. 2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,ARAT,XSAVEOPT,MELTDOWN cpu1: 256KB 64b/line 8-way L2 cache cpu1: smt 0, core 1, package 0 cpu2 at mainbus0: apid 4 (application processor) cpu2: Intel(R) Xeon(R) CPU @ 2.30GHz, 2299.53 MHz, 06-3f-00 cpu2: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,HTT,SSE3,PCLMUL,VMX,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4. 2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,ARAT,XSAVEOPT,MELTDOWN cpu2: 256KB 64b/line 8-way L2 cache cpu2: smt 0, core 2, package 0 cpu3 at mainbus0: apid 6 (application processor) cpu3: Intel(R) Xeon(R) CPU @ 2.30GHz, 2299.54 MHz, 06-3f-00 cpu3: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,HTT,SSE3,PCLMUL,VMX,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4. 2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,ARAT,XSAVEOPT,MELTDOWN cpu3: 256KB 64b/line 8-way L2 cache cpu3: smt 0, core 3, package 0 cpu4 at mainbus0: apid 1 (application processor) cpu4: Intel(R) Xeon(R) CPU @ 2.30GHz, 2299.57 MHz, 06-3f-00 cpu4: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,HTT,SSE3,PCLMUL,VMX,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4. 2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,ARAT,XSAVEOPT,MELTDOWN cpu4: 256KB 64b/line 8-way L2 cache cpu4: smt 1, core 0, package 0 cpu5 at mainbus0: apid 3 (application processor) cpu5: Intel(R) Xeon(R) CPU @ 2.30GHz, 2299.56 MHz, 06-3f-00 cpu5: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,HTT,SSE3,PCLMUL,VMX,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4. 2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,ARAT,XSAVEOPT,MELTDOWN cpu5: 256KB 64b/line 8-way L2 cache cpu5: smt 1, core 1, package 0 cpu6 at mainbus0: apid 5 (application processor) cpu6: Intel(R) Xeon(R) CPU @ 2.30GHz, 2299.54 MHz, 06-3f-00 cpu6: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,HTT,SSE3,PCLMUL,VMX,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4. 2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,ARAT,XSAVEOPT,MELTDOWN cpu6: 256KB 64b/line 8-way L2 cache cpu6: smt 1, core 2, package 0 cpu7 at mainbus0: apid 7 (application processor) cpu7: Intel(R) Xeon(R) CPU @ 2.30GHz, 2299.57 MHz, 06-3f-00 cpu7: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,HTT,SSE3,PCLMUL,VMX,SSSE3,FMA3,CX16,PCID,SSE4.1,SSE4. 2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,PAGE1GB,RDTSCP,LONG,LAHF
Re: top: allow reverse sort order
On Wed, Nov 28, 2018 at 12:07:37AM +0100, Klemens Nanni wrote: > Note how an empty field is silently treated as the default field > "state", but that's an independent issue I'd like to address in a > separate diff for string_index(). Not a problem of string_index() actually. We consistently handle empty input for all commands silently as no-op.
Re: top: allow reverse sort order
On Tue, Nov 27, 2018 at 03:52:52PM -0600, Scott Cheloha wrote: > > > static int > > > +getorder(char *field) > > > +{ > > > + rev_order = field[0] == '-'; > > > + > > > + return string_index(rev_order ? field + 1 : field, statics.order_names); > > > +} > > > + > > You need to check that the field has an ordering before potentially > modifying rev_order, otherwise a bogus input will have a side effect. > > For example, with the current code the input "o -notafield" reverses > the ordering. Good catch. Updated diff to fix that, rest unchanged. Note how an empty field is silently treated as the default field "state", but that's an independent issue I'd like to address in a separate diff for string_index(). OK? Index: display.c === RCS file: /cvs/src/usr.bin/top/display.c,v retrieving revision 1.57 diff -u -p -r1.57 display.c --- display.c 17 Nov 2018 23:10:08 - 1.57 +++ display.c 27 Nov 2018 21:09:56 - @@ -817,7 +817,8 @@ show_help(void) "I | i- toggle the display of idle processes\n" "k [-sig] pid - send signal `-sig' to process `pid'\n" "n|# count- show `count' processes\n" - "o field - specify sort order (size, res, cpu, time, pri, pid, command)\n" + "o [-]field - specify sort order (size, res, cpu, time, pri, pid, command)\n" + " (o -field sorts in reverse)\n" "P pid- highlight process `pid' (P+ switches highlighting off)\n" "p pid- display process by `pid' (p+ selects all processes)\n" "q- quit\n" Index: machine.c === RCS file: /cvs/src/usr.bin/top/machine.c,v retrieving revision 1.95 diff -u -p -r1.95 machine.c --- machine.c 17 Nov 2018 23:10:08 - 1.95 +++ machine.c 27 Nov 2018 21:09:56 - @@ -602,6 +602,8 @@ static unsigned char sorted_state[] = 1 /* zombie*/ }; +extern int rev_order; + /* * proc_compares - comparison functions for "qsort" */ @@ -631,6 +633,17 @@ static unsigned char sorted_state[] = #define ORDERKEY_CMD \ if ((result = strcmp(p1->p_comm, p2->p_comm)) == 0) +/* remove one level of indirection and set sort order */ +#define SETORDER do { \ + if (rev_order) { \ + p1 = *(struct kinfo_proc **) pp2; \ + p2 = *(struct kinfo_proc **) pp1; \ + } else { \ + p1 = *(struct kinfo_proc **) pp1; \ + p2 = *(struct kinfo_proc **) pp2; \ + } \ + } while (0) + /* compare_cpu - the comparison function for sorting by cpu percentage */ static int compare_cpu(const void *v1, const void *v2) @@ -640,9 +653,7 @@ compare_cpu(const void *v1, const void * struct kinfo_proc *p1, *p2; int result; - /* remove one level of indirection */ - p1 = *(struct kinfo_proc **) pp1; - p2 = *(struct kinfo_proc **) pp2; + SETORDER; ORDERKEY_PCTCPU ORDERKEY_CPUTIME @@ -663,9 +674,7 @@ compare_size(const void *v1, const void struct kinfo_proc *p1, *p2; int result; - /* remove one level of indirection */ - p1 = *(struct kinfo_proc **) pp1; - p2 = *(struct kinfo_proc **) pp2; + SETORDER; ORDERKEY_MEM ORDERKEY_RSSIZE @@ -686,9 +695,7 @@ compare_res(const void *v1, const void * struct kinfo_proc *p1, *p2; int result; - /* remove one level of indirection */ - p1 = *(struct kinfo_proc **) pp1; - p2 = *(struct kinfo_proc **) pp2; + SETORDER; ORDERKEY_RSSIZE ORDERKEY_MEM @@ -709,9 +716,7 @@ compare_time(const void *v1, const void struct kinfo_proc *p1, *p2; int result; - /* remove one level of indirection */ - p1 = *(struct kinfo_proc **) pp1; - p2 = *(struct kinfo_proc **) pp2; + SETORDER; ORDERKEY_CPUTIME ORDERKEY_PCTCPU @@ -732,9 +737,7 @@ compare_prio(const void *v1, const void struct kinfo_proc *p1, *p2; int result; - /* remove one level of indirection */ - p1 = *(struct kinfo_proc **) pp1; - p2 = *(struct kinfo_proc **) pp2; + SETORDER; ORDERKEY_PRIO ORDERKEY_PCTCPU @@ -754,9 +757,7 @@ compare_pid(const void *v1, const void * struct kinfo_proc *p1, *p2; int result; - /* remove one level of indirection */ - p1 = *(struct kinfo_proc **) pp1; - p2 = *(struct kinfo_proc **) pp2; + SETORDER; ORDERKEY_PID ORDERKEY_PCTCPU @@ -777,9 +778,7 @@ compare_cmd(const void *v1, const void * struct kinfo_proc *p1, *p2; int result; - /* remove one level of indirection */ - p1 = *(struct kinfo_proc **) pp1; - p2 = *(struct kinfo_
Re: top: allow reverse sort order
On Tue, Nov 27, 2018 at 11:28:31AM -0600, Scott Cheloha wrote: > No objections here to the feature in general. We already support reversing > select orderings in systat(1), which I've found useful in practice, so this > is not without precedent and is consistent with at least one other monitoring > tool in userland. > > I think it's a bit unfortunate that 'r' in interactive mode is already > assigned > to renice the process. If we did that here it'd be consistent with > systat(1)'s > interactive keybindings and a boon for overall UX. But 'r' has mean "renice" > since before top(1) was even imported, so at this late date I don't think we > can > change the binding for 'r'. However, adding an 'R' shortcut to reverse the > ordering > for the current primary key might be a reasonable addition in a later diff. I thought about a toggle as well but went with the "-" prefix semantics since the existing toggles we currently have (`C', `h', `H') are distinct features not related to filters like `g' or `o'. That is, currently you cannot toggle something like "only user kn/all but kn". > > Index: display.c > > === > > RCS file: /cvs/src/usr.bin/top/display.c,v > > retrieving revision 1.57 > > diff -u -p -r1.57 display.c > > --- display.c 17 Nov 2018 23:10:08 - 1.57 > > +++ display.c 24 Nov 2018 14:09:38 - > > @@ -817,7 +817,8 @@ show_help(void) > > "I | i- toggle the display of idle processes\n" > > "k [-sig] pid - send signal `-sig' to process `pid'\n" > > "n|# count- show `count' processes\n" > > - "o field - specify sort order (size, res, cpu, time, pri, pid, > > command)\n" > > + "o [-]field - specify sort order (size, res, cpu, time, pri, pid, > > command)\n" > > + " (o -field sorts in reverse)\n" > > "o" isn't needed, as contextually you're talking about the 'o' shortcut. Keeping "o" is consistent with the rest: "g+ selects all commands" and "u -user hides user" for example. > > @@ -879,10 +877,10 @@ rundisplay(void) > > new_message(MT_standout, > > "Order to sort: "); > > if (readline(tempbuf, sizeof(tempbuf)) > 0) { > > - if ((i = string_index(tempbuf, > > - statics.order_names)) == -1) { > > + if ((i = getorder(tempbuf)) == -1) { > > new_message(MT_standout, > > " %s: unrecognized sorting order", > > + tempbuf[0] == '-' ? tempbuf + 1 : > > tempbuf); > > You aren't ducking the '-' during init so I don't think you should hide it > here. > You could even refactor the two into something like We're also not "ducking" it for `u' in the option parsing. Admittedly, this is inconsistent but I'd like to keep both in the command loop. The command line options are parsed once so with an erroneous `-U --kn' users might see "--kn: unknown user" only once on startup when in fact "-kn" is the unknown user. Stripping the dash in error messages from the command line which is likely used repeatedly at runtime ensures that the correct username is printed. > void setorder(const char *); > > to keep the messages in sync, but that should be done in a subsequent diff. So I if at all, I suggest stripping the dash in the option parsing routine as well for consistency in separate diff.
Re: top: allow reverse sort order
On Tue, Nov 27, 2018 at 11:28:31AM -0600, Scott Cheloha wrote: > On Sat, Nov 24, 2018 at 04:06:34PM +0100, Klemens Nanni wrote: > > Sometimes I want to see certain programs with least amount of memory, > > so this diff implements `o -field' to sort in reverse order. > > > > The logic is straight forward: > > > > 1. merge common code from argument and command loops into new setorder() > > 2. introduce global state `rev_order' (set in the helper) > > 3. move identical code to set up process objects from compare_*() > >functions into SETORDER macro using global boolean > > > > compare_*() are used by qsort(3). To sort in reverse, the macro simply > > swaps the objects used by the ORDERKEY_* macros. That is it inverts the > > comparison from `p1 > p2' into `p2 > p1' respectively `p1 < p2'. > > > > Works fine for all available fields on amd64, no behaviour change for > > "normal" order. > > > > Feedback? Objections? > > [...] After lingering on this I missed a bug in my initial review. See inline. > > Index: display.c > > === > > RCS file: /cvs/src/usr.bin/top/display.c,v > > retrieving revision 1.57 > > diff -u -p -r1.57 display.c > > --- display.c 17 Nov 2018 23:10:08 - 1.57 > > +++ display.c 24 Nov 2018 14:09:38 - > > @@ -817,7 +817,8 @@ show_help(void) > > "I | i- toggle the display of idle processes\n" > > "k [-sig] pid - send signal `-sig' to process `pid'\n" > > "n|# count- show `count' processes\n" > > - "o field - specify sort order (size, res, cpu, time, pri, pid, > > command)\n" > > + "o [-]field - specify sort order (size, res, cpu, time, pri, pid, > > command)\n" > > + " (o -field sorts in reverse)\n" > > "o" isn't needed, as contextually you're talking about the 'o' shortcut. > Maybe "-field reverses order" or "'-' prefix reverses sort order"? > > > "P pid- highlight process `pid' (P+ switches highlighting > > off)\n" > > "p pid- display process by `pid' (p+ selects all > > processes)\n" > > "q- quit\n" > > Index: machine.c > > === > > RCS file: /cvs/src/usr.bin/top/machine.c,v > > retrieving revision 1.95 > > diff -u -p -r1.95 machine.c > > --- machine.c 17 Nov 2018 23:10:08 - 1.95 > > +++ machine.c 24 Nov 2018 14:47:32 - > > @@ -602,6 +602,8 @@ static unsigned char sorted_state[] = > > 1 /* zombie*/ > > }; > > > > +extern int rev_order; > > + > > /* > > * proc_compares - comparison functions for "qsort" > > */ > > @@ -631,6 +633,17 @@ static unsigned char sorted_state[] = > > #define ORDERKEY_CMD \ > > if ((result = strcmp(p1->p_comm, p2->p_comm)) == 0) > > > > +/* remove one level of indirection and set sort order */ > > +#define SETORDER do { \ > > + if (rev_order) { \ > > + p1 = *(struct kinfo_proc **) pp2; \ > > + p2 = *(struct kinfo_proc **) pp1; \ > > + } else { \ > > + p1 = *(struct kinfo_proc **) pp1; \ > > + p2 = *(struct kinfo_proc **) pp2; \ > > + } \ > > + } while (0) > > + > > /* compare_cpu - the comparison function for sorting by cpu percentage */ > > static int > > compare_cpu(const void *v1, const void *v2) > > @@ -640,9 +653,7 @@ compare_cpu(const void *v1, const void * > > struct kinfo_proc *p1, *p2; > > int result; > > > > - /* remove one level of indirection */ > > - p1 = *(struct kinfo_proc **) pp1; > > - p2 = *(struct kinfo_proc **) pp2; > > + SETORDER; > > > > ORDERKEY_PCTCPU > > ORDERKEY_CPUTIME > > @@ -663,9 +674,7 @@ compare_size(const void *v1, const void > > struct kinfo_proc *p1, *p2; > > int result; > > > > - /* remove one level of indirection */ > > - p1 = *(struct kinfo_proc **) pp1; > > - p2 = *(struct kinfo_proc **) pp2; > > + SETORDER; > > > > ORDERKEY_MEM > > ORDERKEY_RSSIZE > > @@ -686,9 +695,7 @@ compare_res(const void *v1, const void * > > struct kinfo_proc *p1, *p2; > > int result; > > > > - /* remove one level of indirection */ > > - p1 = *(struct kinfo_proc **) pp1; > > - p2 = *(struct kinfo_proc **) pp2; > > + SETORDER; > > > > ORDERKEY_RSSIZE > > ORDERKEY_MEM > > @@ -709,9 +716,7 @@ compare_time(const void *v1, const void > > struct kinfo_proc *p1, *p2; > > int result; > > > > - /* remove one level of indirection */ > > - p1 = *(struct kinfo_proc **) pp1; > > - p2 = *(struct kinfo_proc **) pp2; > > + SETORDER; > > > > ORDERKEY_CPUTIME > > ORDERKEY_PCTCPU > > @@ -732,9 +737,7 @@ compare_prio(const void *v1, const void > > struct kinfo_proc *p1, *p2; > > int result; > > > > - /* remove one level of indirection */ > > - p1 = *(struct kinfo_proc **)
mg: toggle-read-only-all
Allow all non-ephemeral buffers to be toggled write/read-only. This toggle works while mg is running, unlike the -R option which is only during mg startup. ok? Index: buffer.c === RCS file: /cvs/src/usr.bin/mg/buffer.c,v retrieving revision 1.104 diff -u -p -u -p -r1.104 buffer.c --- buffer.c6 Aug 2017 04:39:45 - 1.104 +++ buffer.c27 Nov 2018 19:52:41 - @@ -31,6 +31,28 @@ extern int globalwd; /* ARGSUSED */ int +togglereadonlyall(int f, int n) +{ + struct buffer *bp = NULL; + int len = 0; + + allbro = !allbro; + for (bp = bheadp; bp != NULL; bp = bp->b_bufp) { + len = strlen(bp->b_bname); + if (bp->b_bname[0] != '*' && bp->b_bname[len - 1] != '*') { + if (allbro) + bp->b_flag |= BFREADONLY; + else + bp->b_flag &= ~BFREADONLY; + } + } + curwp->w_rflag |= WFMODE; + + return (TRUE); +} + +/* ARGSUSED */ +int togglereadonly(int f, int n) { int s; Index: def.h === RCS file: /cvs/src/usr.bin/mg/def.h,v retrieving revision 1.156 diff -u -p -u -p -r1.156 def.h --- def.h 29 Aug 2018 07:50:16 - 1.156 +++ def.h 27 Nov 2018 19:52:41 - @@ -417,6 +417,7 @@ int delwind(int, int); /* buffer.c */ int togglereadonly(int, int); +int togglereadonlyall(int, int); struct buffer *bfind(const char *, int); int poptobuffer(int, int); int killbuffer(struct buffer *); @@ -737,6 +738,7 @@ extern int defb_flag; extern int doaudiblebell; extern int dovisiblebell; extern int dblspace; +extern int allbro; extern char cinfo[]; extern char*keystrings[]; extern char pat[NPAT]; Index: funmap.c === RCS file: /cvs/src/usr.bin/mg/funmap.c,v retrieving revision 1.54 diff -u -p -u -p -r1.54 funmap.c --- funmap.c29 Aug 2018 07:50:16 - 1.54 +++ funmap.c27 Nov 2018 19:52:41 - @@ -201,6 +201,7 @@ static struct funmap functnames[] = { {usebuffer, "switch-to-buffer",}, {poptobuffer, "switch-to-buffer-other-window",}, {togglereadonly, "toggle-read-only" }, + {togglereadonlyall, "toggle-read-only-all" }, {twiddle, "transpose-chars",}, {transposepara, "transpose-paragraphs",}, {transposeword, "transpose-words",}, Index: main.c === RCS file: /cvs/src/usr.bin/mg/main.c,v retrieving revision 1.84 diff -u -p -u -p -r1.84 main.c --- main.c 16 Sep 2016 17:17:40 - 1.84 +++ main.c 27 Nov 2018 19:52:41 - @@ -28,6 +28,7 @@ intstartrow; /* row to start */ int doaudiblebell; /* audible bell toggle */ int dovisiblebell; /* visible bell toggle */ int dblspace; /* sentence end #spaces */ +int allbro;/* all buffs read-only */ struct buffer *curbp; /* current buffer */ struct buffer *bheadp;/* BUFFER list head */ struct mgwin *curwp; /* current window */ @@ -65,6 +66,7 @@ main(int argc, char **argv) switch (o) { case 'R': bro = 1; + allbro = 1; break; case 'n': nobackups = 1; Index: mg.1 === RCS file: /cvs/src/usr.bin/mg/mg.1,v retrieving revision 1.108 diff -u -p -u -p -r1.108 mg.1 --- mg.118 Nov 2018 07:57:28 - 1.108 +++ mg.127 Nov 2018 19:52:41 - @@ -884,6 +884,10 @@ Prompt and switch to a new buffer in the Switch to buffer in another window. .It toggle-read-only Toggle the read-only flag on the current buffer. +.It toggle-read-only-all +Toggle the read-only flag on all non-ephemeral buffers. +A simple toggle that switches a global read-only flag either on +or off. .It transpose-chars Transpose the two characters in front of and under dot, then move forward one character.
Re: top: allow reverse sort order
On Tue, Nov 27, 2018 at 07:38:59PM +, Jason McIntyre wrote: > On Tue, Nov 27, 2018 at 11:28:31AM -0600, Scott Cheloha wrote: > > > @@ -137,13 +137,16 @@ mode. > > > This is identical to > > > .Em batch > > > mode. > > > -.It Fl o Ar field > > > +.It Fl o Oo - Oc Ns Ar field > > > Sort the process display area using the specified > > > .Ar field > > > as the primary key. > > > The field name is the name of the column as seen in the output, > > > but in lower case. > > > The > > > +.Sq - > > > +prefix reverses the order. > > > > My understanding is that an "order" is the actual progression of > > a sequence, while an "ordering" refers to the underlying function > > or rule that gives rise to a given "order" when applied to a set. > > > > So, I *think* you want > > > > "reverses the ordering for the field" > > > > or something similar, but I'm not positive. > > > > hi. > > i've never heard of "ordering" as a noun like this. i think klemens' > language was fine (and simpler). I can live with that. kn: just make sure the two error messages match up.
Re: top: allow reverse sort order
On Tue, Nov 27, 2018 at 11:28:31AM -0600, Scott Cheloha wrote: > > @@ -137,13 +137,16 @@ mode. > > This is identical to > > .Em batch > > mode. > > -.It Fl o Ar field > > +.It Fl o Oo - Oc Ns Ar field > > Sort the process display area using the specified > > .Ar field > > as the primary key. > > The field name is the name of the column as seen in the output, > > but in lower case. > > The > > +.Sq - > > +prefix reverses the order. > > My understanding is that an "order" is the actual progression of > a sequence, while an "ordering" refers to the underlying function > or rule that gives rise to a given "order" when applied to a set. > > So, I *think* you want > > "reverses the ordering for the field" > > or something similar, but I'm not positive. > hi. i've never heard of "ordering" as a noun like this. i think klemens' language was fine (and simpler). jmc
rad(8): auto discovered prefixes should not overwrite explicitly configured prefixes
say you have 2001:db8:23::1/64 and fdac:23::1/64 configured on ix0 and the following rad.conf: interface ix0 { prefix fdac:23::/64 { autonomous address-configuration no } auto prefix { autonomous address-configuration yes } } rad should announce 2001:db8:23::/64 with the A flag set and fdac:23::/64 without the A flag. Currently it announces both prefixes with the A flag because auto prefix wins. I believe that is the wrong way around. Ross Richardson pointed a use case out in private where you get a dynamic global prefix via dhcpv6-pd and you want a stable ULA prefix for your infrastructure but don't want to use the ULA for slaac. currently that's not possible. OK? diff --git frontend.c frontend.c index 152f13a2738..a02b3ef37ff 100644 --- frontend.c +++ frontend.c @@ -791,10 +791,6 @@ merge_ra_interfaces(void) ra_iface_conf = find_ra_iface_conf( &frontend_conf->ra_iface_list, ra_iface->conf_name); - if (ra_iface_conf->autoprefix) - get_interface_prefixes(ra_iface, - ra_iface_conf->autoprefix); - log_debug("add static prefixes for %s", ra_iface->name); SIMPLEQ_FOREACH(ra_prefix_conf, &ra_iface_conf->ra_prefix_list, @@ -803,6 +799,11 @@ merge_ra_interfaces(void) &ra_prefix_conf->prefix, ra_prefix_conf->prefixlen, ra_prefix_conf); } + + if (ra_iface_conf->autoprefix) + get_interface_prefixes(ra_iface, + ra_iface_conf->autoprefix); + build_packet(ra_iface); } } -- I'm not entirely sure you are real.
Re: bgpd refactor aspath_match a bit
On Tue, Nov 27, 2018 at 06:23:53PM +0100, Claudio Jeker wrote: > On Tue, Nov 27, 2018 at 04:21:53PM +0100, Job Snijders wrote: > > On Fri, Nov 23, 2018 at 03:55:18PM +0100, Claudio Jeker wrote: > > > For origin validation I chacked the source_as in struct rde_aspath > > > this is not really the right place. It should be in struct aspath > > > since that holds all the ASPATH related stuff. Change this, move > > > aspath_match out of util.c back into rde_attr.c and adjust code to use > > > the cached value also in match from any source-as XYZ rules. > > > This last bit causes a minor behavioural change since the old code > > > extracted the last non AS_SET asnumber. The new code follows the ROA > > > RFC and returns the rightmost AS for AS_SEQUENCE, the local AS for > > > empty paths and AS_NONE (which is 0) for everything else. > > > So now 'match from any source-as 0' will return all paths that do not > > > have a final AS_SEQUENCE segment. > > > > > > The reason for this change is that I don't want to have two different > > > behaviours for what we call source-as (the one in roa-set and the one on a > > > filter). > > > > Something is off, it seems 'source-as 0' is matching anything that has > > an AS_SET attribute set: > > > > $ bgpctl show rib source-as 0 | head > > flags: * = Valid, > = Selected, I = via IBGP, A = Announced, > >S = Stale, E = Error > > origin validation state: N = not-found, V = valid, ! = invalid > > origin: i = IGP, e = EGP, ? = Incomplete > > > > flags ovs destination gateway lpref med aspath > > origin > > I*> N 5.39.176.0/21192.147.168.1 100 0 2914 8530 { > > 198753 } ? > > I*> N 5.101.110.0/24 192.147.168.1 100 0 2914 14061 > > { 46652 } i > > I*> N 5.175.0.0/19 192.147.168.1 100 0 2914 1299 > > 20773 { 8972 } i > > I*> N 8.41.202.0/24192.147.168.1 100 0 2914 13789 > > 30372 { 40179 } i > > > > Similarly, this should return at least 5.39.176.0/21: > > > > $ bgpctl show rib source-as 8530 > > flags: * = Valid, > = Selected, I = via IBGP, A = Announced, > >S = Stale, E = Error > > origin validation state: N = not-found, V = valid, ! = invalid > > origin: i = IGP, e = EGP, ? = Incomplete > > > > flags ovs destination gateway lpref med aspath > > origin > > I*> N 80.87.16.0/20192.147.168.1 100 0 2914 8530 ? > > I*> N 87.236.128.0/21 192.147.168.1 100 0 2914 8530 ? > > I*> N 88.151.152.0/21 192.147.168.1 100 0 2914 8530 ? > > I*> N 89.38.120.0/21 192.147.168.1 100 0 2914 8530 i > > I*> N 93.115.176.0/20 192.147.168.1 100 0 2914 8530 i > > I*> N 185.52.144.0/22 192.147.168.1 100 0 2914 8530 ? > > > > I implemented source-as the way ROA is defining it. So anything which ends > with a AS_SET will return AS_NONE (which is 0). OpenBGPD has no way to > have an AS_PATH that has a real 0 in the AS_PATH (those UPDATES are > treated as withdraw). Because of this also the 5.39.176.0/21 is no longer > matching in 'bgpctl show rib source-as 8530'. I'm not sure it should behave that way. 'bgpctl show rib source-as 8530' really ought to return prefixes like 80.87.16.0/20 but also 5.39.176.0/21. > I'm a bit on the edge here about where to go and currently prefer to > follow a RFC (which in this case is RFC6811). > > o Route Origin ASN: The origin AS number derived from a Route as > follows: > > * the rightmost AS in the final segment of the AS_PATH attribute > in the Route if that segment is of type AS_SEQUENCE, or > > * the BGP speaker's own AS number if that segment is of type >AS_CONFED_SEQUENCE or AS_CONFED_SET or if the AS_PATH is empty, >or > > * the distinguished value "NONE" if the final segment of the >AS_PATH attribute is of any other type. > > As mentioned above I found it strange when behaviour is different because > of where it is used. RFC 6811 describes how BGP Origin Validation should be performed, but is not a guideline how to treat AS_SETs in general or how CLIs should function. Perhaps 'source-as 0' should just throw an error in all contexts (both bgpd.conf and bgpctl), since 0 can never be in an AS_SET or AS_SEQUENCE. Perhaps we shouldn't overload the number. 1/ Maybe 'source-as self' or 'source-as none' can be used to match all routes originated by the AS in which this bgpd instance runs. 2/ Similarly, the program could be made so that 'AS 15562' (lets use 15562 as example) from the Global Configuration passed on to bgpctl and used in the filter ruleset means both "what is originated by 15562" and also what is originated in the own AS (and doesn't have AS_PATH yet, but we know the router runs in 15562 because of the global config parameter). Kind regards, Job
Re: top: allow reverse sort order
On Sat, Nov 24, 2018 at 04:06:34PM +0100, Klemens Nanni wrote: > Sometimes I want to see certain programs with least amount of memory, > so this diff implements `o -field' to sort in reverse order. > > The logic is straight forward: > > 1. merge common code from argument and command loops into new setorder() > 2. introduce global state `rev_order' (set in the helper) > 3. move identical code to set up process objects from compare_*() >functions into SETORDER macro using global boolean > > compare_*() are used by qsort(3). To sort in reverse, the macro simply > swaps the objects used by the ORDERKEY_* macros. That is it inverts the > comparison from `p1 > p2' into `p2 > p1' respectively `p1 < p2'. > > Works fine for all available fields on amd64, no behaviour change for > "normal" order. > > Feedback? Objections? I mentioned in private that I disliked the lack of parentheses in the macro invocations, but after rereading I see that that's the style in use in the code already, so you're fine, my bad. No objections here to the feature in general. We already support reversing select orderings in systat(1), which I've found useful in practice, so this is not without precedent and is consistent with at least one other monitoring tool in userland. I think it's a bit unfortunate that 'r' in interactive mode is already assigned to renice the process. If we did that here it'd be consistent with systat(1)'s interactive keybindings and a boon for overall UX. But 'r' has mean "renice" since before top(1) was even imported, so at this late date I don't think we can change the binding for 'r'. However, adding an 'R' shortcut to reverse the ordering for the current primary key might be a reasonable addition in a later diff. Some nits inline for documentation, error messages. Otherwise ok cheloha@. ... and I might be wrong about those nits, too. Definitely have jmc@ weigh in on the nits and the docs changes in general. > Index: display.c > === > RCS file: /cvs/src/usr.bin/top/display.c,v > retrieving revision 1.57 > diff -u -p -r1.57 display.c > --- display.c 17 Nov 2018 23:10:08 - 1.57 > +++ display.c 24 Nov 2018 14:09:38 - > @@ -817,7 +817,8 @@ show_help(void) > "I | i- toggle the display of idle processes\n" > "k [-sig] pid - send signal `-sig' to process `pid'\n" > "n|# count- show `count' processes\n" > - "o field - specify sort order (size, res, cpu, time, pri, pid, > command)\n" > + "o [-]field - specify sort order (size, res, cpu, time, pri, pid, > command)\n" > + " (o -field sorts in reverse)\n" "o" isn't needed, as contextually you're talking about the 'o' shortcut. Maybe "-field reverses order" or "'-' prefix reverses sort order"? > "P pid- highlight process `pid' (P+ switches highlighting > off)\n" > "p pid- display process by `pid' (p+ selects all > processes)\n" > "q- quit\n" > Index: machine.c > === > RCS file: /cvs/src/usr.bin/top/machine.c,v > retrieving revision 1.95 > diff -u -p -r1.95 machine.c > --- machine.c 17 Nov 2018 23:10:08 - 1.95 > +++ machine.c 24 Nov 2018 14:47:32 - > @@ -602,6 +602,8 @@ static unsigned char sorted_state[] = > 1 /* zombie*/ > }; > > +extern int rev_order; > + > /* > * proc_compares - comparison functions for "qsort" > */ > @@ -631,6 +633,17 @@ static unsigned char sorted_state[] = > #define ORDERKEY_CMD \ > if ((result = strcmp(p1->p_comm, p2->p_comm)) == 0) > > +/* remove one level of indirection and set sort order */ > +#define SETORDER do { \ > + if (rev_order) { \ > + p1 = *(struct kinfo_proc **) pp2; \ > + p2 = *(struct kinfo_proc **) pp1; \ > + } else { \ > + p1 = *(struct kinfo_proc **) pp1; \ > + p2 = *(struct kinfo_proc **) pp2; \ > + } \ > + } while (0) > + > /* compare_cpu - the comparison function for sorting by cpu percentage */ > static int > compare_cpu(const void *v1, const void *v2) > @@ -640,9 +653,7 @@ compare_cpu(const void *v1, const void * > struct kinfo_proc *p1, *p2; > int result; > > - /* remove one level of indirection */ > - p1 = *(struct kinfo_proc **) pp1; > - p2 = *(struct kinfo_proc **) pp2; > + SETORDER; > > ORDERKEY_PCTCPU > ORDERKEY_CPUTIME > @@ -663,9 +674,7 @@ compare_size(const void *v1, const void > struct kinfo_proc *p1, *p2; > int result; > > - /* remove one level of indirection */ > - p1 = *(struct kinfo_proc **) pp1; > - p2 = *(struct kinfo_proc **) pp2; > + SETORDER; > > ORDERKEY_MEM > ORDERKEY_RSSIZE > @@ -686,9 +695,7 @@ compare_res(const void *
Re: bgpd refactor aspath_match a bit
On Tue, Nov 27, 2018 at 04:21:53PM +0100, Job Snijders wrote: > Hi Claudio, > > On Fri, Nov 23, 2018 at 03:55:18PM +0100, Claudio Jeker wrote: > > For origin validation I chacked the source_as in struct rde_aspath > > this is not really the right place. It should be in struct aspath > > since that holds all the ASPATH related stuff. Change this, move > > aspath_match out of util.c back into rde_attr.c and adjust code to use > > the cached value also in match from any source-as XYZ rules. > > This last bit causes a minor behavioural change since the old code > > extracted the last non AS_SET asnumber. The new code follows the ROA > > RFC and returns the rightmost AS for AS_SEQUENCE, the local AS for > > empty paths and AS_NONE (which is 0) for everything else. > > So now 'match from any source-as 0' will return all paths that do not > > have a final AS_SEQUENCE segment. > > > > The reason for this change is that I don't want to have two different > > behaviours for what we call source-as (the one in roa-set and the one on a > > filter). > > Something is off, it seems 'source-as 0' is matching anything that has > an AS_SET attribute set: > > $ bgpctl show rib source-as 0 | head > flags: * = Valid, > = Selected, I = via IBGP, A = Announced, >S = Stale, E = Error > origin validation state: N = not-found, V = valid, ! = invalid > origin: i = IGP, e = EGP, ? = Incomplete > > flags ovs destination gateway lpref med aspath origin > I*> N 5.39.176.0/21192.147.168.1 100 0 2914 8530 { > 198753 } ? > I*> N 5.101.110.0/24 192.147.168.1 100 0 2914 14061 { > 46652 } i > I*> N 5.175.0.0/19 192.147.168.1 100 0 2914 1299 > 20773 { 8972 } i > I*> N 8.41.202.0/24192.147.168.1 100 0 2914 13789 > 30372 { 40179 } i > > Similarly, this should return at least 5.39.176.0/21: > > $ bgpctl show rib source-as 8530 > flags: * = Valid, > = Selected, I = via IBGP, A = Announced, >S = Stale, E = Error > origin validation state: N = not-found, V = valid, ! = invalid > origin: i = IGP, e = EGP, ? = Incomplete > > flags ovs destination gateway lpref med aspath origin > I*> N 80.87.16.0/20192.147.168.1 100 0 2914 8530 ? > I*> N 87.236.128.0/21 192.147.168.1 100 0 2914 8530 ? > I*> N 88.151.152.0/21 192.147.168.1 100 0 2914 8530 ? > I*> N 89.38.120.0/21 192.147.168.1 100 0 2914 8530 i > I*> N 93.115.176.0/20 192.147.168.1 100 0 2914 8530 i > I*> N 185.52.144.0/22 192.147.168.1 100 0 2914 8530 ? > I implemented source-as the way ROA is defining it. So anything which ends with a AS_SET will return AS_NONE (which is 0). OpenBGPD has no way to have an AS_PATH that has a real 0 in the AS_PATH (those UPDATES are treated as withdraw). Because of this also the 5.39.176.0/21 is no longer matching in 'bgpctl show rib source-as 8530'. I'm a bit on the edge here about where to go and currently prefer to follow a RFC (which in this case is RFC6811). o Route Origin ASN: The origin AS number derived from a Route as follows: * the rightmost AS in the final segment of the AS_PATH attribute in the Route if that segment is of type AS_SEQUENCE, or * the BGP speaker's own AS number if that segment is of type AS_CONFED_SEQUENCE or AS_CONFED_SET or if the AS_PATH is empty, or * the distinguished value "NONE" if the final segment of the AS_PATH attribute is of any other type. As mentioned above I found it strange when behaviour is different because of where it is used. -- :wq Claudio
Re: bgpd refactor aspath_match a bit
Hi Claudio, On Fri, Nov 23, 2018 at 03:55:18PM +0100, Claudio Jeker wrote: > For origin validation I chacked the source_as in struct rde_aspath > this is not really the right place. It should be in struct aspath > since that holds all the ASPATH related stuff. Change this, move > aspath_match out of util.c back into rde_attr.c and adjust code to use > the cached value also in match from any source-as XYZ rules. > This last bit causes a minor behavioural change since the old code > extracted the last non AS_SET asnumber. The new code follows the ROA > RFC and returns the rightmost AS for AS_SEQUENCE, the local AS for > empty paths and AS_NONE (which is 0) for everything else. > So now 'match from any source-as 0' will return all paths that do not > have a final AS_SEQUENCE segment. > > The reason for this change is that I don't want to have two different > behaviours for what we call source-as (the one in roa-set and the one on a > filter). Something is off, it seems 'source-as 0' is matching anything that has an AS_SET attribute set: $ bgpctl show rib source-as 0 | head flags: * = Valid, > = Selected, I = via IBGP, A = Announced, S = Stale, E = Error origin validation state: N = not-found, V = valid, ! = invalid origin: i = IGP, e = EGP, ? = Incomplete flags ovs destination gateway lpref med aspath origin I*> N 5.39.176.0/21192.147.168.1 100 0 2914 8530 { 198753 } ? I*> N 5.101.110.0/24 192.147.168.1 100 0 2914 14061 { 46652 } i I*> N 5.175.0.0/19 192.147.168.1 100 0 2914 1299 20773 { 8972 } i I*> N 8.41.202.0/24192.147.168.1 100 0 2914 13789 30372 { 40179 } i Similarly, this should return at least 5.39.176.0/21: $ bgpctl show rib source-as 8530 flags: * = Valid, > = Selected, I = via IBGP, A = Announced, S = Stale, E = Error origin validation state: N = not-found, V = valid, ! = invalid origin: i = IGP, e = EGP, ? = Incomplete flags ovs destination gateway lpref med aspath origin I*> N 80.87.16.0/20192.147.168.1 100 0 2914 8530 ? I*> N 87.236.128.0/21 192.147.168.1 100 0 2914 8530 ? I*> N 88.151.152.0/21 192.147.168.1 100 0 2914 8530 ? I*> N 89.38.120.0/21 192.147.168.1 100 0 2914 8530 i I*> N 93.115.176.0/20 192.147.168.1 100 0 2914 8530 i I*> N 185.52.144.0/22 192.147.168.1 100 0 2914 8530 ? Kind regards, Job
Re: bgpd refactor community code
On Thu, Nov 22, 2018 at 05:56:20PM +0100, Claudio Jeker wrote: > On Tue, Nov 13, 2018 at 06:53:55PM +0100, Claudio Jeker wrote: > > This is a large diff that changes the way communities are stored in > > filters and filter_sets. Both standard communities and large communities > > now share the same data structure for lookups and at the same time the > > filters are extended to allow more then one community to match per rule > > (currently the maximum is 3). I did leave extended communities outside for > > now since this diff is already big enough but they will follow in a second > > step. > > > > So now filters like > > deny to any large-community 196618:0:666 large-community 196618:0:3 > > deny to any community 13030:1016 community 13030:5 > > will work and match only if both communities are matched. > > > > As a side effect the bgpctl show rib code is changed and there is no > > longer a limitation that only one of the filter is allowed to be used. > > In other words 'bgpctl show rib as 13030 community 1303:1036' will > > work now. > > > > Apart from that there should be no other visible change. > > > > Please test and report back OK job@
Re: refactor malloc a bit
On Mon, Nov 26, 2018 at 07:36:56AM +0100, Otto Moerbeek wrote: > Hi, > > this refactors the code that find an existsing allocation into a > separate function. The code is currently repeated in three spots. > > Prepatory work to get a somehwat more efficient version of the > "not-my-pool" case. > > Please review and test, works and reads fine. ok
Re: diff: ftpd(8): fix for sign-compare compiler warnings
On Sun, Nov 25, 2018 at 12:32:23AM +0100, Jan Klemkow wrote: > Hi, > > This diff fixes some -Wsign-compare compiler warnings in ftpd(8) by > using the right types for 'i' and 'len'. One warning is left, but I > don't see that it's fixable without suppressing the warning by a cast of > len to size_t. And casting might be controversial in this case?! The diff looks correct to me. If anyone wants to commit ok tb > /usr/src/libexec/ftpd/ftpd.c:2781:11: warning: comparison of integers of > different signs: 'int' and > 'unsigned long' [-Wsign-compare] > if (len >= sizeof(buf) || len == -1) { > ~~~ ^ ~~~ This test is the wrong way around: compare CAVEATS in snprintf(3). There's a ton of unchecked snprintfs in this code. Did you take a look at those?