Re: asmc: constify global product table

2022-10-20 Thread Joerg Jung
On Wed, Oct 19, 2022 at 02:39:33PM +, Klemens Nanni wrote:
> Looking at symbols in /bsd .data again for .rodata candidates:
> 82281b70 l O .data  1688 asmc_prods
> 
> amsc(4) never writes, but a const table requires a const softc member:
> 
> 263 for (i = 0; asmc_prods[i].pr_name && !sc->sc_prod; i++)
> 264 if (!strncasecmp(asmc_prods[i].pr_name, hw_prod,
> 265 strlen(asmc_prods[i].pr_name)))
> 266 sc->sc_prod = _prods[i];
> 267 if (!sc->sc_prod)
> 268 return;
> 
> Builds fine on amd64.  I don't have Apple hardware to test:
> 820ce550 l O .rodata1688 asmc_prods
> 
> 
> Feedback? Objection? OK?

Tested and works for me.

ok jung@

 
> Index: dev/acpi/asmc.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/asmc.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 asmc.c
> --- dev/acpi/asmc.c   21 Dec 2021 20:53:46 -  1.4
> +++ dev/acpi/asmc.c   19 Oct 2022 14:10:01 -
> @@ -75,7 +75,7 @@ struct asmc_softc {
>   bus_space_tag_t  sc_iot;
>   bus_space_handle_t   sc_ioh;
>  
> - struct asmc_prod*sc_prod;
> + const struct asmc_prod  *sc_prod;
>   uint8_t  sc_nfans;  /* number of fans */
>   uint8_t  sc_lightlen;   /* light data len */
>   uint8_t  sc_backlight;  /* keyboard backlight value */
> @@ -119,7 +119,7 @@ const char *asmc_hids[] = {
>   "APP0001", NULL
>  };
>  
> -static struct asmc_prod asmc_prods[] = {
> +static const struct asmc_prod asmc_prods[] = {
>   { "MacBookAir", 1, {
>   "TB0T", "TB1S", "TB1T", "TB2S", "TB2T", "TBXT", "TC0C", "TC0D",
>   "TC0E", "TC0F", "TC0P", "TC1C", "TC1E", "TC2C", "TCFP", "TCGC",
> 



Re: apmd(8): reconnecting AC, not battery

2022-05-30 Thread Joerg Jung



> On 28. May 2022, at 16:31, Jan Stary  wrote:
> 
> On May 28 10:34:35, s...@spacehopper.org wrote:
>> On 2022/05/28 06:52, Jason McIntyre wrote:
>>> On Fri, May 27, 2022 at 07:19:37PM +0200, Jan Stary wrote:
 apmd says:
 
  When the power status changes (battery is connected or disconnected),
  apmd fetches the current status and reports it via syslog(3)
  with logging facility LOG_DAEMON.
 
 Is "battery" really meant here?  Should that be the AC power?
 Batteries are typically not being reconnected while running ...
>> 
>> They certainly can be, either while on external power, or in the case of
>> machines with multiple batteries (several generations of X series ThinkPads
>> have both internal and external batteries, so you can swap without powering
>> down even while on battery power). But yes the common case would be
>> external power.
>> 
 (The manpage calls it "external power", "line current"
 and "AC" interchangably.)
 
Jan
 
>>> 
>>> hi.
>>> 
>>> i think you're right that the text is off. the author possibly meant to
>>> say battery was charging or discharging.
>>> 
>>> anyway, if i don;t hear any reason not to soon, i'll commit your
>>> suggested diff (which i think is simpler and makes sense).
>> 
>> Why be inaccurate when we don't need to be - it's very uncommon to have
>> a machine with a battery with an AC input. "External power" would make
>> more sense.
> 
> I never liked the AC nomenclature either;

Me neither, but I believe this may have originated in the
APM specification which talks about "AC line status” for i.e.
"Get Power Status” function.  Seems like it went from
specification into code (see machine/apmvar.h) and then
into documentation/man page.

> it's battery vs wall socket, not AC vs DC.
> Feel free to change the wording of course.






Re: [patch] httpd remove unused struct

2020-11-18 Thread Joerg Jung
On Sat, May 23, 2020 at 07:47:20PM -0500, Edgar Pettijohn wrote:
> Remove an unused struct from parse.y.

Yes, this seems correct. Updated diff against -current below.

OK?


Index: parse.y
===
RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
retrieving revision 1.120
diff -u -p -r1.120 parse.y
--- parse.y 29 Oct 2020 12:30:52 -  1.120
+++ parse.y 18 Nov 2020 21:42:16 -
@@ -127,10 +127,6 @@ typedef struct {
struct timeval   tv;
struct portrange port;
struct auth  auth;
-   struct {
-   struct sockaddr_storage  ss;
-   char name[HOST_NAME_MAX+1];
-   }addr;
} v;
int lineno;
 } YYSTYPE;



Re: smtpd: relax ORCPT check again

2020-11-18 Thread Joerg Jung
On Wed, Nov 18, 2020 at 08:57:48PM +, gil...@poolp.org wrote:
> November 18, 2020 9:54 PM, "Joerg Jung"  wrote:
> 
> > Hi,
> > 
> > in my opinion revision 1.423 of smtp_session.c went a bit too far.
> > Enforcing that ORCPT has to have domain results in breakage in real
> > world usage. For example, sending from root user cron jobs mails 
> > via Postfix aliased to an address handled by OpenSMTPD will fail.
> > 
> > RFC leaves some room for interpretation here, but interoperability 
> > may be considered important as well. GitHub issue #1084 [1] has
> > some more elaborations and examples, but was opened for the same
> > reason.
> > 
> > Therefore, I propose the following diff below to slightly relax
> > the ORCPT check again by skipping the domain part check. 
> > 
> > Comments, OK?
> > 
> 
> maybe just skip the check ONLY if domain part is empty ?
> 

Like in the diff below and assuming that domain part is always
nul terminated after text_to_mailaddr()?

OK?


Index: smtp_session.c
===
RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
retrieving revision 1.426
diff -u -p -r1.426 smtp_session.c
--- smtp_session.c  24 Apr 2020 11:34:07 -  1.426
+++ smtp_session.c  18 Nov 2020 21:10:36 -
@@ -2583,7 +2583,8 @@ smtp_tx_rcpt_to(struct smtp_tx *tx, cons
 
if (!text_to_mailaddr(>evp.dsn_orcpt, opt) ||
!valid_localpart(tx->evp.dsn_orcpt.user) ||
-   !valid_domainpart(tx->evp.dsn_orcpt.domain)) {
+   (strlen(tx->evp.dsn_orcpt.domain) != 0 &&
+!valid_domainpart(tx->evp.dsn_orcpt.domain))) {
smtp_reply(tx->session,
"553 ORCPT address syntax error");
return;



smtpd: relax ORCPT check again

2020-11-18 Thread Joerg Jung
Hi,

in my opinion revision 1.423 of smtp_session.c went a bit too far.
Enforcing that ORCPT has to have domain results in breakage in real
world usage.  For example, sending from root user cron jobs mails 
via Postfix aliased to an address handled by OpenSMTPD will fail.

RFC leaves some room for interpretation here, but interoperability 
may be considered important as well.  GitHub issue #1084 [1] has
some more elaborations and examples, but was opened for the same
reason.

Therefore, I propose the following diff below to slightly relax
the ORCPT check again by skipping the domain part check. 

Comments, OK?

Thanks,
Regards,
Joerg

[1] https://github.com/OpenSMTPD/OpenSMTPD/issues/1084



Index: smtp_session.c
===
RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
retrieving revision 1.426
diff -u -p -r1.426 smtp_session.c
--- smtp_session.c  24 Apr 2020 11:34:07 -  1.426
+++ smtp_session.c  18 Nov 2020 20:24:28 -
@@ -2582,8 +2582,7 @@ smtp_tx_rcpt_to(struct smtp_tx *tx, cons
opt += 7;
 
if (!text_to_mailaddr(>evp.dsn_orcpt, opt) ||
-   !valid_localpart(tx->evp.dsn_orcpt.user) ||
-   !valid_domainpart(tx->evp.dsn_orcpt.domain)) {
+   !valid_localpart(tx->evp.dsn_orcpt.user)) {
smtp_reply(tx->session,
"553 ORCPT address syntax error");
return;



Re: acpiapplesmc(4)

2020-09-10 Thread Joerg Jung


> Am 07.09.2020 um 20:52 schrieb Mark Kettenis :
>> Date: Mon, 7 Sep 2020 19:59:13 +0200
>> From: Marcus Glocker 
>> On Mon, 7 Sep 2020 19:25:00 +0200 (CEST)
>> Mark Kettenis  wrote:
 Date: Mon, 7 Sep 2020 12:02:15 -0500
 From: joshua stein 
 
 On Mon, 07 Sep 2020 at 06:58:01 +0200, Marcus Glocker wrote:  
> This is an initial driver for the Apple System Management
> Controller found in Intel based Apple computers.
> 
> The driver is currently missing support for the Sudden Motion
> Sensor (SMS), light sensor, and keyboard backlight since I don't
> have that hardware available to develop on.
> 
> On my iMac11,2 it can deliver fan and temperatures values:
> 
>hw.sensors.acpiapplesmc0.temp0=24.00 degC (Airflow 1)
>hw.sensors.acpiapplesmc0.temp1=33.00 degC (CPU Core 0)
>hw.sensors.acpiapplesmc0.temp2=36.00 degC (CPU Heatsink)
>hw.sensors.acpiapplesmc0.temp3=40.00 degC (CPU Core 1)
>hw.sensors.acpiapplesmc0.temp4=47.00 degC (GPU)
>hw.sensors.acpiapplesmc0.temp5=45.00 degC (GPU Heatsink)
>hw.sensors.acpiapplesmc0.temp6=59.00 degC (PCH)
>hw.sensors.acpiapplesmc0.temp7=42.00 degC (Memory)
>hw.sensors.acpiapplesmc0.temp8=45.00 degC (Mainboard
> Proximity) hw.sensors.acpiapplesmc0.fan0=998 RPM
>hw.sensors.acpiapplesmc0.fan1=1132 RPM
>hw.sensors.acpiapplesmc0.fan2=1198 RPM
> 
> Feedback, testers, OKs?  
 
 Are there machines where asmc(4) will also attach?  

Yes, there are. As far as I know, most of them.

>>> Good point.  My old Macmini1,1 has:
>>> 
>>> ...
>>> "APP0001" at acpi0 not configured
>>> ...
>>> asmc0 at isa0 port 0x300/32: rev 1.3f503, 137 keys
>>> ...
>>> 
>>> So yes, I'd say there are.
>>> 
>>> 
>>> Having an acpi attachment is probably better than doing isa probes.

Yes, agreed.

>>> But we probably should consolidate the drivers.
>> 
>> D'oh!  I wasn't even aware that we already have an asmc(4) driver in our
>> tree.  Shame on me :-|
>> 
>> Glancing over asmc(4) I don't think there is anything more that my
>> driver would support other than attaching over acpi(4).  Would it be
>> possible to only write an acpi glue which attaches to asmc(4)?
> 
> I think we'd just want to turn it into an acpi(4) driver.  Or maybe
> dump it in favour of your driver.

Please, can we make sure the new driver supports light sensors 
and keyboard backlights as the current driver does, before you
dump it?!  
Otherwise that would be an unnecessarily bad regression.

Regards,
Joerg




Re: acpiapplesmc(4)

2020-09-10 Thread Joerg Jung


> Am 10.09.2020 um 23:28 schrieb Marcus Glocker :
> 
> On Thu, 10 Sep 2020 23:07:15 +0200
> Joerg Jung  wrote:
> 
>>> Am 07.09.2020 um 20:52 schrieb Mark Kettenis
>>> :  
>>>> Date: Mon, 7 Sep 2020 19:59:13 +0200
>>>> From: Marcus Glocker 
>>>> On Mon, 7 Sep 2020 19:25:00 +0200 (CEST)
>>>> Mark Kettenis  wrote:  
>>>>>> Date: Mon, 7 Sep 2020 12:02:15 -0500
>>>>>> From: joshua stein 
>>>>>> 
>>>>>> On Mon, 07 Sep 2020 at 06:58:01 +0200, Marcus Glocker wrote:
>>>>>>> This is an initial driver for the Apple System Management
>>>>>>> Controller found in Intel based Apple computers.
>>>>>>> 
>>>>>>> The driver is currently missing support for the Sudden Motion
>>>>>>> Sensor (SMS), light sensor, and keyboard backlight since I don't
>>>>>>> have that hardware available to develop on.
>>>>>>> 
>>>>>>> On my iMac11,2 it can deliver fan and temperatures values:
>>>>>>> 
>>>>>>>   hw.sensors.acpiapplesmc0.temp0=24.00 degC (Airflow 1)
>>>>>>>   hw.sensors.acpiapplesmc0.temp1=33.00 degC (CPU Core 0)
>>>>>>>   hw.sensors.acpiapplesmc0.temp2=36.00 degC (CPU Heatsink)
>>>>>>>   hw.sensors.acpiapplesmc0.temp3=40.00 degC (CPU Core 1)
>>>>>>>   hw.sensors.acpiapplesmc0.temp4=47.00 degC (GPU)
>>>>>>>   hw.sensors.acpiapplesmc0.temp5=45.00 degC (GPU Heatsink)
>>>>>>>   hw.sensors.acpiapplesmc0.temp6=59.00 degC (PCH)
>>>>>>>   hw.sensors.acpiapplesmc0.temp7=42.00 degC (Memory)
>>>>>>>   hw.sensors.acpiapplesmc0.temp8=45.00 degC (Mainboard
>>>>>>> Proximity) hw.sensors.acpiapplesmc0.fan0=998 RPM
>>>>>>>   hw.sensors.acpiapplesmc0.fan1=1132 RPM
>>>>>>>   hw.sensors.acpiapplesmc0.fan2=1198 RPM
>>>>>>> 
>>>>>>> Feedback, testers, OKs?
>>>>>> 
>>>>>> Are there machines where asmc(4) will also attach?
>> 
>> Yes, there are. As far as I know, most of them.
>> 
>>>>> Good point.  My old Macmini1,1 has:
>>>>> 
>>>>> ...
>>>>> "APP0001" at acpi0 not configured
>>>>> ...
>>>>> asmc0 at isa0 port 0x300/32: rev 1.3f503, 137 keys
>>>>> ...
>>>>> 
>>>>> So yes, I'd say there are.
>>>>> 
>>>>> 
>>>>> Having an acpi attachment is probably better than doing isa
>>>>> probes.  
>> 
>> Yes, agreed.
>> 
>>>>> But we probably should consolidate the drivers.  
>>>> 
>>>> D'oh!  I wasn't even aware that we already have an asmc(4) driver
>>>> in our tree.  Shame on me :-|
>>>> 
>>>> Glancing over asmc(4) I don't think there is anything more that my
>>>> driver would support other than attaching over acpi(4).  Would it
>>>> be possible to only write an acpi glue which attaches to asmc(4)?  
>>> 
>>> I think we'd just want to turn it into an acpi(4) driver.  Or maybe
>>> dump it in favour of your driver.  
>> 
>> Please, can we make sure the new driver supports light sensors 
>> and keyboard backlights as the current driver does, before you
>> dump it?!  
>> Otherwise that would be an unnecessarily bad regression.
> 
> As already mentioned, it was not my intention to replace asmc(4).  It
> just don't attaches to my iMac currently, and then I was assuming that
> we have no support for SMC without checking further.  If I would have
> noticed before that we already have asmc(4) in the tree I wouldn't come
> up with a new driver :-)
> 
> Also as already mentioned before I have no other Apple gear with light
> sensor nor keyboard backlight, so no point for me trying to write that
> code.
> 
> So I would say we just leave it as is.
> 
> Thanks all for your feedback anyway.

Don’t give up so quickly ;) 
let’s try to make the driver work on your iMac, send me dmesg and
sysctl hw output please.

Your idea of converting it to ACPI is the right thing to do anyways,
would be nice to get this working.


Re: acpiapplesmc(4)

2020-09-10 Thread Joerg Jung



> Am 10.09.2020 um 14:09 schrieb Marcus Glocker :
> 
> On Mon, 7 Sep 2020 21:39:55 +0200
> Marcus Glocker  wrote:
> 
>> On Mon, 7 Sep 2020 20:50:20 +0200 (CEST)
>> Mark Kettenis  wrote:
>> 
 Date: Mon, 7 Sep 2020 19:59:13 +0200
 From: Marcus Glocker 
 
 On Mon, 7 Sep 2020 19:25:00 +0200 (CEST)
 Mark Kettenis  wrote:
 
>> Date: Mon, 7 Sep 2020 12:02:15 -0500
>> From: joshua stein 
>> 
>> On Mon, 07 Sep 2020 at 06:58:01 +0200, Marcus Glocker wrote:
>> 
>>> This is an initial driver for the Apple System Management
>>> Controller found in Intel based Apple computers.
>>> 
>>> The driver is currently missing support for the Sudden
>>> Motion Sensor (SMS), light sensor, and keyboard backlight
>>> since I don't have that hardware available to develop on.
>>> 
>>> On my iMac11,2 it can deliver fan and temperatures values:
>>> 
>>>hw.sensors.acpiapplesmc0.temp0=24.00 degC (Airflow
>>> 1) hw.sensors.acpiapplesmc0.temp1=33.00 degC (CPU Core 0)
>>>hw.sensors.acpiapplesmc0.temp2=36.00 degC (CPU
>>> Heatsink) hw.sensors.acpiapplesmc0.temp3=40.00 degC (CPU
>>> Core 1) hw.sensors.acpiapplesmc0.temp4=47.00 degC (GPU)
>>>hw.sensors.acpiapplesmc0.temp5=45.00 degC (GPU
>>> Heatsink) hw.sensors.acpiapplesmc0.temp6=59.00 degC (PCH)
>>>hw.sensors.acpiapplesmc0.temp7=42.00 degC (Memory)
>>>hw.sensors.acpiapplesmc0.temp8=45.00 degC (Mainboard
>>> Proximity) hw.sensors.acpiapplesmc0.fan0=998 RPM
>>>hw.sensors.acpiapplesmc0.fan1=1132 RPM
>>>hw.sensors.acpiapplesmc0.fan2=1198 RPM
>>> 
>>> Feedback, testers, OKs?  
>> 
>> Are there machines where asmc(4) will also attach?  
> 
> Good point.  My old Macmini1,1 has:
> 
> ...
> "APP0001" at acpi0 not configured
> ...
> asmc0 at isa0 port 0x300/32: rev 1.3f503, 137 keys
> ...
> 
> So yes, I'd say there are.
> 
> 
> Having an acpi attachment is probably better than doing isa
> probes. But we probably should consolidate the drivers.
 
 D'oh!  I wasn't even aware that we already have an asmc(4) driver
 in our tree.  Shame on me :-|
 
 Glancing over asmc(4) I don't think there is anything more that my
 driver would support other than attaching over acpi(4).  Would it
 be possible to only write an acpi glue which attaches to asmc(4)?
 
>>> 
>>> I think we'd just want to turn it into an acpi(4) driver.  Or maybe
>>> dump it in favour of your driver.  
>> 
>> Ok.  I'll give it a try to convert asmc(4) in to an acpi(4) driver and
>> see how it works here.
> 
> I can make asmc(4) attach through acpi(4) on my machine, but then it
> crashes with uvm fault because it doesn't seem to recognize my machine
> type, and tries to access a NULL pointer on 'sc_prod'.

What is hw_prod on your machine, e.g. please can you
show sysctl hw output and dmesg, please?

> My approach was more to not relay on specific machine types, but just
> check what sensors are supported by the SMC and try to use them.  I
> lack a bit motivation to add specific models to asmc(4) to be honest.

Actually, I’ve chosen this route, because SMC also advertises sensors
which should not be attached, e.g. light sensors not actually available.
Even worse, the other way around: it has several hidden non-advertised
features and different based on the models.

So, one needs dependency list somewhere between model and features.
I tried to keep it as short and simple as possible, and easy enough to update.

> One question just out of curiosity;  When you use acpiapplesmc(4) on
> your iMac, does it support any sensors there, or even does it work at
> all?

I never got feedback/dmesg for a working iMac and don’t own one myself
for testing. So the support in the driver is untested.
Likely my best-(educated-)guessed prod string is just wrong  ;)

Regards,
Jörg


Re: pf: route-to least-states

2020-07-23 Thread Joerg Jung


> On 23. Jul 2020, at 13:23, YASUOKA Masahiko  wrote:
> 
> The diff fixes 2 problems of "least-states":
> 
> - states whose address is selected by sticky-address is not counted
>  for the number of states.
> - interface is not selected properly if selected table entry specifies
>  an interface.
> 
> ok?

Good catch, diff looks good to me. 

ok jung@


> Increase state counter for least-states when the address is selected
> by sticky-address.  Also fix the problem that the interface which is
> specified by the selected table entry is not used properly.
> 
> Index: sys/net/pf_lb.c
> ===
> RCS file: /disk/cvs/openbsd/src/sys/net/pf_lb.c,v
> retrieving revision 1.64
> diff -u -p -r1.64 pf_lb.c
> --- sys/net/pf_lb.c   2 Jul 2019 09:04:53 -   1.64
> +++ sys/net/pf_lb.c   23 Jul 2020 11:06:05 -
> @@ -97,6 +97,8 @@ u_int64_tpf_hash(struct pf_addr *, st
> intpf_get_sport(struct pf_pdesc *, struct pf_rule *,
>   struct pf_addr *, u_int16_t *, u_int16_t,
>   u_int16_t, struct pf_src_node **);
> +int   pf_map_addr_states_increase(sa_family_t,
> + struct pf_pool *, struct pf_addr *);
> intpf_get_transaddr_af(struct pf_rule *,
>   struct pf_pdesc *, struct pf_src_node **);
> intpf_map_addr_sticky(sa_family_t, struct pf_rule *,
> @@ -319,6 +321,12 @@ pf_map_addr_sticky(sa_family_t af, struc
>   sns[type] = NULL;
>   return (-1);
>   }
> +
> + if ((rpool->opts & PF_POOL_TYPEMASK) == PF_POOL_LEASTSTATES) {
> + if (pf_map_addr_states_increase(af, rpool, naddr) == -1)
> + return (-1);
> + }
> +
>   if (!PF_AZERO(cached, af))
>   pf_addrcpy(naddr, cached, af);
>   if (pf_status.debug >= LOG_DEBUG) {
> @@ -345,6 +353,7 @@ pf_map_addr(sa_family_t af, struct pf_ru
>   struct pf_addr   faddr;
>   struct pf_addr  *raddr = >addr.v.a.addr;
>   struct pf_addr  *rmask = >addr.v.a.mask;
> + struct pfi_kif  *kif;
>   u_int64_tstates;
>   u_int16_tweight;
>   u_int64_tload;
> @@ -539,6 +548,7 @@ pf_map_addr(sa_family_t af, struct pf_ru
> 
>   states = rpool->states;
>   weight = rpool->weight;
> + kif = rpool->kif;
> 
>   if ((rpool->addr.type == PF_ADDR_TABLE &&
>   rpool->addr.p.tbl->pfrkt_refcntcost > 0) ||
> @@ -581,6 +591,7 @@ pf_map_addr(sa_family_t af, struct pf_ru
>   if (cload < load) {
>   states = rpool->states;
>   weight = rpool->weight;
> + kif = rpool->kif;
>   load = cload;
> 
>   pf_addrcpy(naddr, >counter, af);
> @@ -591,29 +602,10 @@ pf_map_addr(sa_family_t af, struct pf_ru
>   } while (pf_match_addr(1, , rmask, >counter, af) &&
>   (states > 0));
> 
> - if (rpool->addr.type == PF_ADDR_TABLE) {
> - if (pfr_states_increase(rpool->addr.p.tbl,
> - naddr, af) == -1) {
> - if (pf_status.debug >= LOG_DEBUG) {
> - log(LOG_DEBUG,"pf: pf_map_addr: "
> - "selected address ");
> - pf_print_host(naddr, 0, af);
> - addlog(". Failed to increase count!\n");
> - }
> - return (1);
> - }
> - } else if (rpool->addr.type == PF_ADDR_DYNIFTL) {
> - if (pfr_states_increase(rpool->addr.p.dyn->pfid_kt,
> - naddr, af) == -1) {
> - if (pf_status.debug >= LOG_DEBUG) {
> - log(LOG_DEBUG, "pf: pf_map_addr: "
> - "selected address ");
> - pf_print_host(naddr, 0, af);
> - addlog(". Failed to increase count!\n");
> - }
> - return (1);
> - }
> - }
> + if (pf_map_addr_states_increase(af, rpool, naddr) == -1)
> + return (1);
> + /* revert the kif which was set by pfr_pool_get() */
> + rpool->kif = kif;
>   break;
>   }
> 
> @@ -642,6 +634,38 @@ pf_map_addr(sa_family_t af, struct pf_ru
>   addlog("\n");
>   }
> 
> + return (0);
> +}
> +
> +int
> +pf_map_addr_states_increase(sa_family_t af, struct pf_pool *rpool,
> +struct pf_addr *naddr)
> +{
> 

Re: smtpd stricter forkmda()

2020-05-04 Thread Joerg Jung


> On 4. May 2020, at 11:17, Gilles Chehade  wrote:
> 
> forkmda() is never supposed to be called with an action dispatcher which
> is not local, this would indicate that the code path was abused somehow.
> 
> idea suggested by Demi M. Obenour


ok jung@ (for post-lock)

> diff --git a/smtpd/smtpd.c b/smtpd/smtpd.c
> index ce1262fa..4c5fc3d9 100644
> --- a/smtpd/smtpd.c
> +++ b/smtpd/smtpd.c
> @@ -1409,6 +1409,8 @@ forkmda(struct mproc *p, uint64_t id, struct deliver 
> *deliver)
>   const char  *pw_dir;
> 
>   dsp = dict_xget(env->sc_dispatchers, deliver->dispatcher);
> + if (dsp->type != DISPATCHER_LOCAL)
> + fatalx("non-local dispatcher called from forkmda()");
> 
>   log_debug("debug: smtpd: forking mda for session %016"PRIx64
>   ": %s as %s", id, deliver->userinfo.username,



Re: smtpd: fix catch-all in virtual aliases

2020-04-28 Thread Joerg Jung


> On 28. Apr 2020, at 10:10, gil...@poolp.org wrote:
> April 28, 2020 8:55 AM, "Joerg Jung" mailto:m...@umaxx.net>> 
> wrote:
> 
>> Also this change might break existing valid setups (e.g. with mailing list
>> servers), but people will likely know how to cope with it.
> 
> Do you have an example of an existing valid setup that is broken with this ?

No example, I just presumed (that’s why I wrote “might”).



Re: smtpd: fix catch-all in virtual aliases

2020-04-28 Thread Joerg Jung


> On 26. Apr 2020, at 18:30, Eric Faurot  wrote:
> 
> When a catch-all entry (@) is used in a virtual alias table, it
> eventually (and mistakenly) catches everything that expands to a
> username. For example, with:
> 
>f...@example.com  user
>@catchall
> 
> "f...@example.com" expands to "user" as expected, but then "user"
> expands to "catchall" because it is interpreted as "user@" (empty
> domain).

Which makes sense to me. If one doesn’t specify a domain after the ‘@‘,
I would expect to really catch-all for all domains and all users. 

> The catch-all fallback mechanism is really meant for full email
> addresses in virtual context, and should not happen for usernames.
> The following diff fixes it.

Yes, I agree that catch-all only really meant to be used for single virtual
domain context and not with primary domains. 

But instead of allowing the syntax and ignoring the case in aliases.c
as in your diff below, I would prefer to “fail" on parsing of the table and 
error logging that an empty domain after ‘@‘ is not a valid syntax, no?

Also this change might break existing valid setups (e.g. with mailing list 
servers), but people will likely know how to cope with it. 

Regards,
Joerg

> Index: aliases.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/aliases.c,v
> retrieving revision 1.77
> diff -u -p -r1.77 aliases.c
> --- aliases.c 28 Dec 2018 12:47:28 -  1.77
> +++ aliases.c 26 Apr 2020 16:04:51 -
> @@ -164,6 +164,10 @@ aliases_virtual_get(struct expand *expan
>   if (ret)
>   goto expand;
> 
> + /* Do not try catch-all entries if there is no domain */
> + if (domain[0] == '\0')
> + return 0;
> +
>   if (!bsnprintf(buf, sizeof(buf), "@%s", domain))
>   return 0;
>   /* Failed ? We lookup for catch all for virtual domain */
> 



Re: smtpd: fix report event format

2020-04-08 Thread Joerg Jung


> On 8. Apr 2020, at 17:19, Eric Faurot  wrote:
> 
> Some users had issues with report events for MAIL FROM and RCPT TO
> when "|" appear in the mail address (yes, it seems to happen), because
> that's also the field separator.  To make parsing the report lines a
> bit more straightforward, it's better to put the address as the last
> field.

While this obviously would fix things, I wonder if there is a better choice for 
the separator available, to avoid similar issues in future.
Something that does not show up in hostnames, mail from, rcpt to, or 
anything else SMTP (filter) protocol related.
Maybe better use ‘$' or even a control character like RS/US record/unit 
separator (ASCII no 29/30)?

> Note that this is a protocol change, so external filters will have
> to be updated.
> 
> Eric.
> 
> Index: lka_filter.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
> retrieving revision 1.60
> diff -u -p -r1.60 lka_filter.c
> --- lka_filter.c  8 Jan 2020 01:41:11 -   1.60
> +++ lka_filter.c  4 Apr 2020 08:39:19 -
> @@ -35,7 +35,7 @@
> #include "smtpd.h"
> #include "log.h"
> 
> -#define  PROTOCOL_VERSION"0.5"
> +#define  PROTOCOL_VERSION"0.6"
> 
> struct filter;
> struct filter_session;
> @@ -1526,7 +1526,7 @@ lka_report_smtp_tx_mail(const char *dire
>   break;
>   }
>   report_smtp_broadcast(reqid, direction, tv, "tx-mail", "%08x|%s|%s\n",
> - msgid, address, result);
> + msgid, result, address);
> }
> 
> void
> @@ -1546,7 +1546,7 @@ lka_report_smtp_tx_rcpt(const char *dire
>   break;
>   }
>   report_smtp_broadcast(reqid, direction, tv, "tx-rcpt", "%08x|%s|%s\n",
> - msgid, address, result);
> + msgid, result, address);
> }
> 
> void
> 



Re: [patch] smtpd: fix for ctype casts

2020-02-24 Thread Joerg Jung


> On 24. Feb 2020, at 20:31, Todd C. Miller  wrote:
> 
> I have a mostly-identical patch in my tree, though I tried to improve
> readability a bit.

ok jung@

> - todd
> 
> Index: usr.sbin/smtpd/mta_session.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/mta_session.c,v
> retrieving revision 1.132
> diff -u -p -u -r1.132 mta_session.c
> --- usr.sbin/smtpd/mta_session.c  24 Feb 2020 16:16:07 -  1.132
> +++ usr.sbin/smtpd/mta_session.c  24 Feb 2020 18:19:22 -
> @@ -1295,13 +1295,12 @@ mta_io(struct io *io, int evt, void *arg
>   if (s->replybuf[0] == '\0')
>   (void)strlcat(s->replybuf, line, sizeof 
> s->replybuf);
>   else if (len > 4) {
> - line = line + 4;
> - if (isdigit((int)*line) && *(line + 1) == '.' &&
> - isdigit((int)*line+2) && *(line + 3) == '.' 
> &&
> - isdigit((int)*line+4) && 
> isspace((int)*(line + 5)))
> - (void)strlcat(s->replybuf, line+5, 
> sizeof s->replybuf);
> - else
> - (void)strlcat(s->replybuf, line, sizeof 
> s->replybuf);
> + p = line + 4;
> + if (isdigit((unsigned char)p[0]) && p[1] == '.' 
> &&
> + isdigit((unsigned char)p[2]) && p[3] == '.' 
> &&
> + isdigit((unsigned char)p[4]) && 
> isspace((unsigned char)p[5]))
> + p += 5;
> + (void)strlcat(s->replybuf, p, sizeof 
> s->replybuf);
>   }
>   goto nextline;
>   }
> @@ -1313,9 +1312,9 @@ mta_io(struct io *io, int evt, void *arg
>   (void)strlcat(s->replybuf, line, sizeof s->replybuf);
>   else if (len > 4) {
>   p = line + 4;
> - if (isdigit((int)*p) && *(p + 1) == '.' &&
> - isdigit((int)*p+2) && *(p + 3) == '.' &&
> - isdigit((int)*p+4) && isspace((int)*(p + 5)))
> + if (isdigit((unsigned char)p[0]) && p[1] == '.' &&
> + isdigit((unsigned char)p[2]) && p[3] == '.' &&
> + isdigit((unsigned char)p[4]) && isspace((unsigned 
> char)p[5]))
>   p += 5;
>   if (strlcat(s->replybuf, p, sizeof s->replybuf) >= 
> sizeof s->replybuf)
>   (void)strlcpy(s->replybuf, line, sizeof 
> s->replybuf);
> Index: usr.sbin/smtpd/parse.y
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v
> retrieving revision 1.276
> diff -u -p -u -r1.276 parse.y
> --- usr.sbin/smtpd/parse.y3 Feb 2020 15:41:22 -   1.276
> +++ usr.sbin/smtpd/parse.y24 Feb 2020 18:19:51 -
> @@ -529,7 +529,7 @@ SMTP LIMIT limits_smtp
>   free($3);
>   YYERROR;
>   }
> - if (isspace((int)*$3) ||  !isprint((int)*$3) || *$3== '@') {
> + if (isspace((unsigned char)*$3) || !isprint((unsigned char)*$3) || *$3 
> == '@') {
>   yyerror("sub-addr-delim uses invalid character");
>   free($3);
>   YYERROR;
> Index: usr.sbin/smtpd/smtp_client.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtp_client.c,v
> retrieving revision 1.12
> diff -u -p -u -r1.12 smtp_client.c
> --- usr.sbin/smtpd/smtp_client.c  10 Sep 2019 12:08:26 -  1.12
> +++ usr.sbin/smtpd/smtp_client.c  24 Feb 2020 18:21:43 -
> @@ -779,9 +779,10 @@ smtp_client_replycat(struct smtp_client 
>   line += 3;
>   if (line[0]) {
>   line += 1;
> - if (isdigit((int)line[0]) && line[1] == '.' &&
> - isdigit((int)line[2]) && line[3] == '.' &&
> - isdigit((int)line[4]) && isspace((int)line[5]))
> + if (isdigit((unsigned char)line[0]) && line[1] == '.' &&
> + isdigit((unsigned char)line[2]) && line[3] == '.' &&
> + isdigit((unsigned char)line[4]) &&
> + isspace((unsigned char)line[5]))
>   line += 5;
>   }
>   } else
> Index: usr.sbin/smtpd/util.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/util.c,v
> retrieving revision 1.150
> diff -u -p -u -r1.150 util.c
> --- usr.sbin/smtpd/util.c 3 Oct 2019 04:49:12 -   1.150
> +++ usr.sbin/smtpd/util.c 24 Feb 2020 19:17:12 -
> @@ -462,7 +462,7 @@ valid_domainpart(const char *s)
>   

Re: locate.updatedb TMPDIR

2020-02-09 Thread Joerg Jung
On Sun, Feb 09, 2020 at 04:33:13PM +0100, Ingo Schwarze wrote:
>
> this is absolutely not OK.
> 
> How did you test this?

I changed weekly directly on the remote machine to detect and narrow 
down the issue and generated the diff later on my local machine source
tree. 

>$ doas cat /var/log/weekly.part  
>   /etc/weekly[79]: no closing quote

Hah! Thanks for spotting this! Below the correct diff.
OK? 

FYI, this effectively reverts Revision 1.9 committed by mickey about 20
years ago.

> > Note, this might break existing setups, where one has set TMPDIR in
> > /etc/weekly.local to point elsewhere.  This might be handled with a
> > -current upgrade entry?

Please find below another possible diff for current.html
OK?


Index: etc/weekly
===
RCS file: /cvs/src/etc/weekly,v
retrieving revision 1.29
diff -u -p -r1.29 weekly
--- etc/weekly  30 Dec 2019 16:49:51 -  1.29
+++ etc/weekly  9 Feb 2020 19:51:31 -
@@ -48,7 +48,7 @@ if [ -f /var/db/locate.database ]; then
if TMP=`mktemp /var/db/locate.database.XX`; then
trap 'rm -f $TMP; exit 1' 0 1 15
UPDATEDB="/usr/libexec/locate.updatedb"
-   echo "${UPDATEDB} --fcodes=- --tmpdir=${TMPDIR:-/tmp}" | \
+   echo "${UPDATEDB} --fcodes=-" | \
nice -5 su -m nobody 1>$TMP
if [ $? -ne 0 ]; then
echo "Rebuilding locate database failed"





Index: faq/current.html
===
RCS file: /cvs/www/faq/current.html,v
retrieving revision 1.1025
diff -u -p -r1.1025 current.html
--- faq/current.html8 Feb 2020 10:19:11 -   1.1025
+++ faq/current.html9 Feb 2020 19:51:35 -
@@ -205,8 +205,17 @@ Old binaries should be deleted and scrip
 adapted.
 
 
-# rm -f /usr/sbin/{dig,host,nslookup}
+# rm -f /usr/sbin/{dig,host,nslookup}
 
+
+
+2020/02/09 - /etc/weekly locate.updatedb TMPDIR
+
+TMPDIR is no longer propagated for locate.updatedb 
+in https://man.openbsd.org/weekly.8;>weekly(8).
+Custom TMPDIR values for locate.updatedb set in 
+root crontab or /etc/weekly.local should be moved into
+/etc/locate.rc.
 
 
 

Re: locate.updatedb TMPDIR

2020-02-09 Thread Joerg Jung
On Sun, Feb 09, 2020 at 04:18:52PM +0100, Ingo Schwarze wrote:
> Hi Todd,
> 
> Todd C. Miller wrote on Sun, Feb 09, 2020 at 07:52:10AM -0700:
> 
> > I'm fine with this.
> 
> I don't really object, but i'm not sure it is needed either.
> 
> It's certainly obvious that command line arguments override defaults.
> That's what they always do.  It's their whole point, in general.
> 
> Also, command line arguments (at least almost) always override
> configuration file settings.  If they don't, i would usually call
> that a serious design error.  There may be exceptions, like
> configuration settings to enable or disable optional command
> line functionality, but that is very unusual.

Personally, I won't necessarily expect that, as I have seen way to many
tools doing all kinds of weird things with ordering, e.g. defaults,
overridden by local user config, overridden by current env, overridden
by global config, overridden by command line flags, etc.  In this
particular case I had to lookup the order in the code to narrow down the
actual issue.  Hence I thought it would be a good idea to document this.

However, I'm fine with assuming sane defaults on OpenBSD, so not needed
then. 

> > However, it doesn't look like we document /etc/locate.rc anywhere.
> > I'm not sure it deserves its own man page
> > but I think it should at least be documented in locate.updatedb
> > in some fashion.
> 
> It is mentioned there, but not really documented.
> 
> OK?

OK jung@

>   Ingo
> 
> 
> Index: locate.updatedb.8
> ===
> RCS file: /cvs/src/usr.bin/locate/locate/locate.updatedb.8,v
> retrieving revision 1.18
> diff -u -r1.18 locate.updatedb.8
> --- locate.updatedb.8 10 Dec 2009 00:45:43 -  1.18
> +++ locate.updatedb.8 9 Feb 2020 15:14:57 -
> @@ -51,10 +51,6 @@
>  .Pa /etc/weekly
>  script.
>  .Pp
> -The contents of the newly built database can be controlled by the
> -.Pa /etc/locate.rc
> -file as well as the command line arguments.
> -.Pp
>  The options are as follows:
>  .Bl -tag -width Ds
>  .It Fl -fcodes
> @@ -75,6 +71,14 @@
>  .It Fl -tmpdir
>  Sets the directory temporary files are stored in.
>  .El
> +.Pp
> +The default settings are controlled by the configuration file
> +.Pa /etc/locate.rc .
> +It is a
> +.Xr sh 1
> +script that can be used to set variables.
> +The names of the variables match the names of the command line
> +options, but in all caps.
>  .Sh FILES
>  .Bl -tag -width /var/db/locate.database -compact
>  .It Pa /etc/locate.rc
> 



Re: locate.updatedb TMPDIR

2020-02-08 Thread Joerg Jung
On Sun, Feb 09, 2020 at 12:33:42AM +0100, Joerg Jung wrote:
> Hi,
> 
> I have a machine with a large data storage attached, but it has only 2GB
> of /tmp (which I consider enough usually).  On this machine weekly
> locate.updatedb fails, due to /tmp being full.  To fix this I would like
> to point locate to a different TMPDIR.
>  
> But it seems one can not just set TMPDIR from /etc/locate.rc to point
> elsewhere, since command line args in /etc/weekly override
> /etc/locate.rc settings.

Below is a second diff, which tries to document that behaviour.
 
> While it's possible to add a weekly.local to set TMPDIR I believe it
> should be better set in the actual configuration file instead of 
> ignoring locate.rc.
> 
> Thus the diff below proposes to remove command line argument to be able
> to use /etc/locate.rc instead.
> 
> Note, this might break existing setups, where one has set TMPDIR in
> /etc/weekly.local to point elsewhere.  This might be handled with a
> -current upgrade entry?
> 
> Comments? OK?
> 
> Thanks,
> Regards,
> Joerg
> 
> 
> Index: etc/weekly
> ===
> RCS file: /cvs/src/etc/weekly,v
> retrieving revision 1.29
> diff -u -p -r1.29 weekly
> --- etc/weekly30 Dec 2019 16:49:51 -  1.29
> +++ etc/weekly8 Feb 2020 23:13:02 -
> @@ -48,7 +48,7 @@ if [ -f /var/db/locate.database ]; then
>   if TMP=`mktemp /var/db/locate.database.XX`; then
>   trap 'rm -f $TMP; exit 1' 0 1 15
>   UPDATEDB="/usr/libexec/locate.updatedb"
> - echo "${UPDATEDB} --fcodes=- --tmpdir=${TMPDIR:-/tmp}" | \
> + echo "${UPDATEDB} --fcodes=- | \
>   nice -5 su -m nobody 1>$TMP
>   if [ $? -ne 0 ]; then
>   echo "Rebuilding locate database failed"
> 


Index: usr.bin/locate/locate/locate.updatedb.8
===
RCS file: /cvs/src/usr.bin/locate/locate/locate.updatedb.8,v
retrieving revision 1.18
diff -u -p -r1.18 locate.updatedb.8
--- usr.bin/locate/locate/locate.updatedb.8 10 Dec 2009 00:45:43 -  
1.18
+++ usr.bin/locate/locate/locate.updatedb.8 8 Feb 2020 23:44:41 -
@@ -54,6 +54,7 @@ script.
 The contents of the newly built database can be controlled by the
 .Pa /etc/locate.rc
 file as well as the command line arguments.
+Command line arguments override rc file and defaults.
 .Pp
 The options are as follows:
 .Bl -tag -width Ds



locate.updatedb TMPDIR

2020-02-08 Thread Joerg Jung
Hi,

I have a machine with a large data storage attached, but it has only 2GB
of /tmp (which I consider enough usually).  On this machine weekly
locate.updatedb fails, due to /tmp being full.  To fix this I would like
to point locate to a different TMPDIR.
 
But it seems one can not just set TMPDIR from /etc/locate.rc to point
elsewhere, since command line args in /etc/weekly override
/etc/locate.rc settings.

While it's possible to add a weekly.local to set TMPDIR I believe it
should be better set in the actual configuration file instead of 
ignoring locate.rc.

Thus the diff below proposes to remove command line argument to be able
to use /etc/locate.rc instead.

Note, this might break existing setups, where one has set TMPDIR in
/etc/weekly.local to point elsewhere.  This might be handled with a
-current upgrade entry?

Comments? OK?

Thanks,
Regards,
Joerg


Index: etc/weekly
===
RCS file: /cvs/src/etc/weekly,v
retrieving revision 1.29
diff -u -p -r1.29 weekly
--- etc/weekly  30 Dec 2019 16:49:51 -  1.29
+++ etc/weekly  8 Feb 2020 23:13:02 -
@@ -48,7 +48,7 @@ if [ -f /var/db/locate.database ]; then
if TMP=`mktemp /var/db/locate.database.XX`; then
trap 'rm -f $TMP; exit 1' 0 1 15
UPDATEDB="/usr/libexec/locate.updatedb"
-   echo "${UPDATEDB} --fcodes=- --tmpdir=${TMPDIR:-/tmp}" | \
+   echo "${UPDATEDB} --fcodes=- | \
nice -5 su -m nobody 1>$TMP
if [ $? -ne 0 ]; then
echo "Rebuilding locate database failed"



Re: get rid of almost empty /etc/examples/mixerctl.conf

2020-02-08 Thread Joerg Jung
On Sat, Feb 08, 2020 at 03:33:37PM -0700, Theo de Raadt wrote:
> Jason McIntyre  wrote:
> 
> > without getting into a discussion about /etc/examples, in this case i
> > personally see neither the point of the example config file (so trivial
> > as to be questionable) nor the addition to the man page (if the example
> > is worthwhile, add it to mixerctl, not the conf page).
> 
> there is additional pieces of "documentation" that occurs here
> 
> - this is a filename, which if placed one directory above., would
>   perform a function
> - the comment format of the file is demonstrated
> - the basic format of the file is demonstrated (is it simply a series
>   of name=value, or have we created a richer DSL)

  - points out common use case
 
> you can argue that is in the manual page.  It is.  After you read a
> lot more.  There is a pace-of-learning going on here.  Noone ever
> demanded that learning must come from a man page, additional sources
> of transient information are not prohibited, but I'm picking up a
> disdain for information learned any other way...

I agree with that, personally I find /etc/examples/ helpful.



Re: smtpd: remove implicit listen on socket

2019-11-27 Thread Joerg Jung



> On 26. Nov 2019, at 07:44, Gilles Chehade  wrote:
> 
> hello,
> 
> smtpd has an implicit listener which is "listen on socket".
> 
> I propose that we write it explicitely in the default config and give up
> with this last bit of implicit configuration.
> 
> The goal behind that is to stop having implicit behaviors but it is also
> to improve security in the daemon:
> 
> OpenSMTPD uses /var/run/smtpd.sock both as a control socket AND enqueuer
> socket, which means that socket is rw-rw-rw- and the control process has
> the charge of checking uid of caller and if permission is allowed to run
> a specific command.
> 
> I think we should really have a control socket and one/many SMTP sockets
> so the control socket could be given tigher filesystem permissions while
> we could also allow multiple enqueue sockets with different permissions,
> and control them through the smtpd.conf ruleset like we do for any other
> connection.
> 
> The first step towards that is this diff.
> 
> ok ?

ok jung@

> Index: smtpd.conf
> ===
> RCS file: /cvs/src/etc/mail/smtpd.conf,v
> retrieving revision 1.13
> diff -u -p -r1.13 smtpd.conf
> --- smtpd.conf25 Nov 2019 13:30:04 -  1.13
> +++ smtpd.conf26 Nov 2019 06:27:11 -
> @@ -5,6 +5,8 @@
> 
> table aliases file:/etc/mail/aliases
> 
> +listen on socket
> +
> # To accept external mail, replace with: listen on all
> #
> listen on lo0
> 
> 
> 
> -- 
> Gilles Chehade   @poolpOrg
> 
> https://www.poolp.orgpatreon: https://www.patreon.com/gilles
> 



remove waf from port-modules(5)

2017-05-27 Thread Joerg Jung
Hi,

I think devel/waf is gone since two years and may not come back, so no
need to mention in port-modules(5).

OK?

Regards,
Joerg



Index: share/man/man5/port-modules.5
===
RCS file: /cvs/src/share/man/man5/port-modules.5,v
retrieving revision 1.217
diff -u -p -r1.217 port-modules.5
--- share/man/man5/port-modules.5   22 Apr 2017 14:00:30 -  1.217
+++ share/man/man5/port-modules.5   27 May 2017 19:37:38 -
@@ -795,26 +795,6 @@ It provides a
 and
 .Cm do-install
 targets that can be overridden in the port Makefile.
-.It devel/waf
-Adds
-.Pa devel/waf
-to
-.Ev BUILD_DEPENDS ,
-.Pa lang/python
-to
-.Ev MODULES ,
-and provides
-.Cm do-configure ,
-.Cm do-build ,
-.Cm do-install
-and
-.Cm post-install
-targets.
-.Cm do-build ,
-.Cm do-install
-and
-.Cm post-install
-can be overridden in the port Makefile.
 .It font
 .It fortran
 Sets



Re: Improved support for Apple trackpads: tests needed

2017-03-11 Thread Joerg Jung
On Fri, Mar 10, 2017 at 12:47:37AM +0100, Ulf Brosziewski wrote:
> This patch for ubcmtp makes it use the multitouch-input functions of
> wsmouse. It's the first driver that would apply the "tracking" variant
> (wsmouse_mtframe).
> 
> No wonders will result from the change, but the two-finger gestures that
> involve movement - scrolling and click-and-drag with two fingers on a
> clickpad - should work without flaws.
> 
> A quick way to check whether all touch positions are available and the
> selection of the active touch works properly is two-finger-scrolling,
> performed with only one finger moving, then switching to the other one.
> 
> Please note that click-and-drag will still require that the "ClickPad"
> attribute is set in the synaptics(4) configuration.
> 
> The patch has been tested on a MacBookPro 8,2 (from 2011). It would be
> nice to learn that it doesn't introduce regressions on older or newer
> models.
> 
> If you don't use a brand-new version of the synaptics driver, you may
> encounter the following bug: If the X cursor is in a window with
> scrollable content and you put two finger on the touchpad without moving
> them, the window content may quickly scroll up and down by a small
> distance. This bug is fixed in the latest version.


Tested and works fine MacBookAir6,2 with some slightly tweaked values:

synclient ClickFinger2=3
synclient ClickFinger3=2
synclient ClickPad=1
synclient VertScrollDelta=-237
synclient HorizScrollDelta=-237
synclient HorizTwoFingerScroll=1

ClickPad/click-and-drag  works, awesome! :)

Diff reads fine. Thanks!
OK jung@ 

> Index: dev/usb/ubcmtp.c
> ===
> RCS file: /cvs/src/sys/dev/usb/ubcmtp.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 ubcmtp.c
> --- dev/usb/ubcmtp.c  30 Mar 2016 23:34:12 -  1.12
> +++ dev/usb/ubcmtp.c  9 Mar 2017 22:17:50 -
> @@ -125,6 +125,12 @@ struct ubcmtp_finger {
>  #define UBCMTP_SN_COORD  250
>  #define UBCMTP_SN_ORIENT 10
>  
> +/* Identify clickpads in ubcmtp_configure. */
> +#define IS_CLICKPAD(ubcmtp_type) (ubcmtp_type != UBCMTP_TYPE1)
> +
> +/* Use a constant, synaptics-compatible pressure value for now. */
> +#define DEFAULT_PRESSURE 40
> +
>  struct ubcmtp_limit {
>   int limit;
>   int min;
> @@ -316,17 +322,6 @@ static struct ubcmtp_dev ubcmtp_devices[
>   },
>  };
>  
> -/* coordinates for each tracked finger */
> -struct ubcmtp_pos {
> - int down;
> - int x;
> - int y;
> - int z;
> - int w;
> - int dx;
> - int dy;
> -};
> -
>  struct ubcmtp_softc {
>   struct device   sc_dev; /* base device */
>  
> @@ -355,7 +350,8 @@ struct ubcmtp_softc {
>   uint32_tsc_status;
>  #define UBCMTP_ENABLED   1
>  
> - struct ubcmtp_pos   pos[UBCMTP_MAX_FINGERS];
> + struct mtpoint  frame[UBCMTP_MAX_FINGERS];
> + int contacts;
>   int btn;
>  };
>  
> @@ -519,6 +515,24 @@ ubcmtp_activate(struct device *self, int
>  }
>  
>  int
> +ubcmtp_configure(struct ubcmtp_softc *sc)
> +{
> + struct wsmousehw *hw = wsmouse_get_hw(sc->sc_wsmousedev);
> +
> + hw->type = WSMOUSE_TYPE_ELANTECH;   /* see ubcmtp_ioctl */
> + hw->hw_type = (IS_CLICKPAD(sc->dev_type->type)
> + ? WSMOUSEHW_CLICKPAD : WSMOUSEHW_TOUCHPAD);
> + hw->x_min = sc->dev_type->l_x.min;
> + hw->x_max = sc->dev_type->l_x.max;
> + hw->y_min = sc->dev_type->l_y.min;
> + hw->y_max = sc->dev_type->l_y.max;
> + hw->mt_slots = UBCMTP_MAX_FINGERS;
> + hw->flags = WSMOUSEHW_MT_TRACKING;
> +
> + return wsmouse_configure(sc->sc_wsmousedev, NULL, 0);
> +}
> +
> +int
>  ubcmtp_enable(void *v)
>  {
>   struct ubcmtp_softc *sc = v;
> @@ -534,6 +548,9 @@ ubcmtp_enable(void *v)
>   return (1);
>   }
>  
> + if (ubcmtp_configure(sc))
> + return (1);
> +
>   if (ubcmtp_setup_pipes(sc) == 0) {
>   sc->sc_status |= UBCMTP_ENABLED;
>   return (0);
> @@ -608,6 +625,7 @@ ubcmtp_ioctl(void *v, unsigned long cmd,
>   wsmode);
>   return (EINVAL);
>   }
> + wsmouse_set_mode(sc->sc_wsmousedev, wsmode);
>  
>   DPRINTF("%s: changing mode to %s\n",
>   sc->sc_dev.dv_xname, (wsmode == WSMOUSE_COMPAT ? "compat" :
> @@ -779,7 +797,7 @@ ubcmtp_tp_intr(struct usbd_xfer *xfer, v
>   struct ubcmtp_softc *sc = priv;
>   struct ubcmtp_finger *pkt;
>   u_int32_t pktlen;
> - int i, diff = 0, finger = 0, fingers = 0, s, t;
> + int i, s, btn, contacts, fingers;
>  
>   if (usbd_is_dying(sc->sc_udev) || !(sc->sc_status & UBCMTP_ENABLED))
>   return;
> @@ -803,76 +821,30 @@ ubcmtp_tp_intr(struct usbd_xfer *xfer, v
>   pkt = (struct ubcmtp_finger *)(sc->tp_pkt + sc->tp_offset);
>   fingers = (pktlen - 

Re: aml_rdpciaddr is busted

2016-10-20 Thread Joerg Jung
On Wed, Oct 19, 2016 at 09:51:47PM +0200, Mark Kettenis wrote:
> The bus number it reports will be totally bogus for devices behind PCI
> bridges.  As a consequence AML will peek and poke at registers of the
> wrong device.  This is what caused the suspend issues with Joris'
> Macbook.
> 
> The diff below attempts to fix this by using the mapping between AML
> nodes and PCI devices that we establish.  Because _INI methods may end
> up executing AML that ends up calling aml_rdpciaddr(), the mapping is
> now established before we execute _INI.  I'm not 100% sure this is
> correct as there is a possibility that _INI will poke some PCI bridges
> in a way that changes the mapping.  Only one way to find out...
> 
> The code also deals with devices that aren't there in a slightly
> different way.  They are now included in the node -> PCI mapping, but
> they're still not included in the list of PCI device nodes that we
> create.  Writing to config space for devices that aren't there is
> implemented as a no-op.  Reading will return an all-ones pattern.
> 
> So far I've only tested this on my x1.  More testing is needed.

Tested the diff together with the timer diff from Joris on MacBookAir6,2.
Does no blow up the machine. pci/ppb lines in dmesg change with this 
(less Thunderbolt lines), dmesg diff below.

Regards,
Joerg


--- dmesg.old   Thu Oct 20 19:32:44 2016
+++ dmesg.new   Thu Oct 20 19:31:51 2016
@@ -1,4 +1,4 @@
-/bsd: OpenBSD 6.0-current (GENERIC.MP) #0: Wed Oct 19 20:30:41 CEST 2016
+/bsd: OpenBSD 6.0-current (GENERIC.MP) #0: Thu Oct 20 19:18:57 CEST 2016
 /bsd: y...@marvin.hq.umaxx.net:/usr/src/sys/arch/amd64/compile/GENERIC.MP
 /bsd: RTC BIOS diagnostic error 
ff
 /bsd: real mem = 8509276160 (8115MB)
@@ -17,7 +17,7 @@
 /bsd: acpihpet0 at acpi0: 14318179 Hz
 /bsd: acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
 /bsd: cpu0 at mainbus0: apid 0 (boot processor)
-/bsd: cpu0: Intel(R) Core(TM) i7-4650U CPU @ 1.70GHz, 1600.21 MHz
+/bsd: cpu0: Intel(R) Core(TM) i7-4650U CPU @ 1.70GHz, 1600.28 MHz
 /bsd: cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,SENSOR,ARAT
 /bsd: cpu0: 256KB 64b/line 8-way L2 cache
 /bsd: cpu0: smt 0, core 0, package 0
@@ -25,17 +25,17 @@
 /bsd: cpu0: apic clock running at 100MHz
 /bsd: cpu0: mwait min=64, max=64, C-substates=0.2.1.2.4.1.1.1, IBE
 /bsd: cpu1 at mainbus0: apid 2 (application processor)
-/bsd: cpu1: Intel(R) Core(TM) i7-4650U CPU @ 1.70GHz, 1600.00 MHz
+/bsd: cpu1: Intel(R) Core(TM) i7-4650U CPU @ 1.70GHz, 1600.01 MHz
 /bsd: cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,SENSOR,ARAT
 /bsd: cpu1: 256KB 64b/line 8-way L2 cache
 /bsd: cpu1: smt 0, core 1, package 0
 /bsd: cpu2 at mainbus0: apid 1 (application processor)
-/bsd: cpu2: Intel(R) Core(TM) i7-4650U CPU @ 1.70GHz, 1600.00 MHz
+/bsd: cpu2: Intel(R) Core(TM) i7-4650U CPU @ 1.70GHz, 1600.01 MHz
 /bsd: cpu2: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,SENSOR,ARAT
 /bsd: cpu2: 256KB 64b/line 8-way L2 cache
 /bsd: cpu2: smt 1, core 0, package 0
 /bsd: cpu3 at mainbus0: apid 3 (application processor)
-/bsd: cpu3: Intel(R) Core(TM) i7-4650U CPU @ 1.70GHz, 1600.00 MHz
+/bsd: cpu3: Intel(R) Core(TM) i7-4650U CPU @ 1.70GHz, 1600.01 MHz
 /bsd: cpu3: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,SENSOR,ARAT
 /bsd: cpu3: 256KB 64b/line 8-way L2 cache
 /bsd: cpu3: smt 1, core 1, package 0
@@ -92,22 +92,9 @@
 /bsd: "Broadcom BCM4360" rev 0x03 at pci3 dev 0 function 0 not configured
 /bsd: ppb3 at pci0 dev 28 function 4 "Intel 8 Series PCIE" rev 0xe4: msi
 /bsd: pci4 at ppb3 bus 5
-/bsd: ppb4 at pci4 dev 0 function 0 "Intel DSL3510 Thunderbolt" rev 0x03
-/bsd: pci5 at ppb4 bus 6
-/bsd: ppb5 at pci5 dev 0 function 0 

Re: video(1): use _NET_WM_STATE_FULLSCREEN

2016-09-25 Thread Joerg Jung

Am 26.09.2016 um 00:11 schrieb Dmitrij D. Czarkoff :

>> The diff below fixes fullscreen mode on window managers that prevent
>> applications from resizing their windows.  

For example, which WM?

>> This is done by setting
>> _NET_WM_STATE_FULLSCREEN atom.
> 
> Ping.

Shouldn't the WM fixed instead?

> --
> Dmitrij D. Czarkoff
> 
> Index: video.c
> ===
> RCS file: /var/cvs/xenocara/app/video/video.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 video.c
> --- video.c6 Jun 2016 19:31:22 -   1.19
> +++ video.c18 Sep 2016 21:33:29 -
> @@ -160,6 +160,7 @@ struct video {
>  int   conv_type;
>  int   enc;
>  int   full_screen;
> +  int  net_wm;
>  int   width;
>  int   height;
>  int   bpf;
> @@ -178,6 +179,7 @@ int xv_get_info(struct video *);
> int xv_sel_adap(struct video *);
> void xv_dump_info(struct video *);
> int xv_init(struct video *);
> +void net_wm_supported(struct video *);
> void resize_window(struct video *, int);
> void display_event(struct video *);
> 
> @@ -500,34 +502,84 @@ xv_init(struct video *vid)
> }
> 
> void
> +net_wm_supported(struct video *vid)
> +{
> +  struct xdsp *x = >xdsp;
> +  Atom query, fullscreen;
> +  Atom type;
> +  Atom *data;
> +  long off;
> +  int fmt, len = 12;
> +  unsigned long nitems, remain;
> +  int i;
> +
> +  query = XInternAtom(x->dpy, "_NET_SUPPORTED", True);
> +  fullscreen = XInternAtom(x->dpy, "_NET_WM_STATE_FULLSCREEN", \
> True);
> +
> +  for (off = 0; XGetWindowProperty(x->dpy, x->rwin, query, off,
> +len, False, XA_ATOM, , , \
> , ,
> +(unsigned char **)) == Success; \
> off += len) {
> +  if (type == XA_ATOM && fmt == 32) {
> +  for (i = 0; i < nitems; i++) {
> +  if (data[i] == fullscreen) {
> +  vid->net_wm = 1;
> +  XFree(data);
> +  return;
> +  }
> +  }
> +  }
> +  XFree(data);
> +  }
> +
> +  return;
> +}
> +
> +void
> resize_window(struct video *vid, int fullscreen)
> {
>  struct xdsp *x = >xdsp;
>  XWindowAttributes winatt;
>  Window junk;
> -  int new_width;
> -  int new_height;
>  int new_x;
>  int new_y;
> +  XEvent ev;
> +  Atom property, value;
> 
>  if (fullscreen == 1) {
>   if (vid->full_screen == 1) {
> -  new_width = x->saved_w;
> -  new_height = x->saved_h;
> +  x->width = x->saved_w;
> +  x->height = x->saved_h;
>new_x = x->saved_x;
>new_y = x->saved_y;
> -  vid->full_screen = 0;
> } else {
> -  new_width = x->max_width;
> -  new_height = x->max_height;
> +  x->width = x->max_width;
> +  x->height = x->max_height;
>new_x = 0;
>new_y = 0;
> -  vid->full_screen = 1;
> }
> -  x->width = new_width;
> -  x->height = new_height;
> -  XMoveResizeWindow(x->dpy, x->window, new_x, new_y,
> -  new_width, new_height);
> +
> +  vid->full_screen = !vid->full_screen;
> +
> +  if (vid->net_wm) {
> +  property = XInternAtom(x->dpy, "_NET_WM_STATE", \
> False);
> +  value = XInternAtom(x->dpy, "_NET\
> _WM_STATE_FULLSCREEN",
> +  False);
> +
> +  memset(, 0, sizeof(ev));
> +  ev.type = ClientMessage;
> +  ev.xclient.window = x->window;
> +  ev.xclient.message_type = property;
> +  ev.xclient.format = 32;
> +  ev.xclient.data.l[0] = vid->full_screen;
> +  ev.xclient.data.l[1] = value;
> +
> +  XSendEvent(x->dpy, x->rwin, False,
> +  SubstructureNotifyMask | Subs\
> tructureRedirectMask,
> +  );
> +  } else {
> +  XMoveResizeWindow(x->dpy, x->window, x->width,
> +  x->height, new_x, new_y);
> +  }
> } else if (!vid->full_screen) {
>   XGetWindowAttributes(x->dpy, x->window, );
>   XTranslateCoordinates(x->dpy, x->window, x->rwin,
> @@ -536,8 +588,8 @@ resize_window(struct video *vid, int ful
>   x->saved_h = x->height = winatt.height;
>   x->saved_x = new_x;
>   x->saved_y = new_y;
> -  XResizeWindow(x->dpy, x->window, x->width, x->height);
> }
> +
>  XSync(x->dpy, False);
>  XSync(x->dpy, True);
> }
> @@ -1435,6 +1487,9 @@ setup(struct video *vid)
>   if (!mmap_init(vid))
>return 0;
> }
> +
> +

Re: smtpd config parsing cleanup

2016-09-09 Thread Joerg Jung

> On 09 Sep 2016, at 13:35, Eric Faurot  wrote:
> 
> Because of the small ad hoc changes that were made here and there over
> the years, the listener config code has become a bit convoluted I think.

Yes.

> Here is a little cleanup to:
> 
> - have all listener creation functions take listen_opts as param,
>  and call config_listener() when done, which adds the listener(s)
>  to the current config list of listeners.
> 
> - make the fallback chain between interface(), host_v4() host_v6()
>  and host_dns() obvious when creating an if_listener.
> 
> - fix a bug where the specified family was ignored if the listener
>  is given as a hostname.
> 
> 
> Comments?

I like that. ok jung@

> Eric.
> 
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v
> retrieving revision 1.189
> diff -u -p -r1.189 parse.y
> --- parse.y   31 Aug 2016 15:24:04 -  1.189
> +++ parse.y   9 Sep 2016 11:11:54 -
> @@ -138,15 +138,14 @@ static struct listen_opts {
>   uint32_toptions;
> } listen_opts;
> 
> -static struct listener   *create_sock_listener(struct listen_opts *);
> -static void   create_if_listener(struct listenerlist *,  struct 
> listen_opts *);
> -static void   config_listener(struct listener *,  struct listen_opts 
> *);
> -
> -struct listener  *host_v4(const char *, in_port_t);
> -struct listener  *host_v6(const char *, in_port_t);
> -int   host_dns(struct listenerlist *, struct listen_opts *);
> -int   host(struct listenerlist *, struct listen_opts *);
> -int   interface(struct listenerlist *, struct listen_opts *);
> +static void  create_sock_listener(struct listen_opts *);
> +static void  create_if_listener(struct listen_opts *);
> +static void  config_listener(struct listener *, struct listen_opts *);
> +static int   host_v4(struct listen_opts *);
> +static int   host_v6(struct listen_opts *);
> +static int   host_dns(struct listen_opts *);
> +static int   interface(struct listen_opts *);
> +
> void   set_local(const char *);
> void   set_localaddrs(struct table *);
> intdelaytonum(char *);
> @@ -695,13 +694,13 @@ socket_listener : SOCKET sock_listen {
>   yyerror("socket listener already configured");
>   YYERROR;
>   }
> - conf->sc_sock_listener = 
> create_sock_listener(_opts);
> + create_sock_listener(_opts);
>   }
>   ;
> 
> if_listener   : STRING if_listen {
>   listen_opts.ifx = $1;
> - create_if_listener(conf->sc_listeners, _opts);
> + create_if_listener(_opts);
>   }
>   ;
> 
> @@ -2005,7 +2004,7 @@ parse_config(struct smtpd *x_conf, const
>   /* If the socket listener was not configured, create a default one. */
>   if (!conf->sc_sock_listener) {
>   memset(_opts, 0, sizeof listen_opts);
> - conf->sc_sock_listener = create_sock_listener(_opts);
> + create_sock_listener(_opts);
>   }
> 
>   /* Free macros and check which have not been used. */
> @@ -2109,7 +2108,7 @@ symget(const char *nam)
>   return (NULL);
> }
> 
> -static struct listener *
> +static void
> create_sock_listener(struct listen_opts *lo)
> {
>   struct listener *l = xcalloc(1, sizeof(*l), "create_sock_listener");
> @@ -2118,13 +2117,12 @@ create_sock_listener(struct listen_opts 
>   l->ss.ss_family = AF_LOCAL;
>   l->ss.ss_len = sizeof(struct sockaddr *);
>   l->local = 1;
> + conf->sc_sock_listener = l;
>   config_listener(l, lo);
> -
> - return (l);
> }
> 
> static void
> -create_if_listener(struct listenerlist *ll,  struct listen_opts *lo)
> +create_if_listener(struct listen_opts *lo)
> {
>   uint16_tflags;
> 
> @@ -2142,17 +2140,11 @@ create_if_listener(struct listenerlist *
>   if (lo->port) {
>   lo->flags = lo->ssl|lo->auth|flags;
>   lo->port = htons(lo->port);
> - if (!interface(ll, lo))
> - if (host(ll, lo) <= 0)
> - errx(1, "invalid virtual ip or interface: %s", 
> lo->ifx);
>   }
>   else {
>   if (lo->ssl & F_SMTPS) {
>   lo->port = htons(465);
>   lo->flags = F_SMTPS|lo->auth|flags;
> - if (!interface(ll, lo))
> - if (host(ll, lo) <= 0)
> - errx(1, "invalid virtual ip or 
> interface: %s", lo->ifx);
>   }
> 
>   if (!lo->ssl || (lo->ssl & F_STARTTLS)) {
> @@ -2160,11 +2152,19 @@ create_if_listener(struct listenerlist *
>   lo->flags = lo->auth|flags;
>   if (lo->ssl & F_STARTTLS)
>   lo->flags |= 

Re: Some cleanups and tweaks for wc(1)

2016-09-07 Thread Joerg Jung

> On 04 Sep 2016, at 00:06, Frederic Cambus  wrote:
> 
> Hi tech@,
> 
> Some cleanups and tweaks for wc(1):
> 
> - Removed unnecessary string.h include
> - Changed 'format_and_print' argument type to int64_t and casting
>  inside the function
> - Declaring 'print_counts', 'format_and_print', and 'cnt' as static
> - Remove unnecessary cast for NULL, and (void) casts from printfs,
>  'mbtowc' and 'format_and_print' calls
> - In 'cnt', change bufsz type from ssize_t to size_t to avoid
>  converting between pointers to integer types with different sign
>  when calling getline (catched when compiling with Clang)
> - Use return instead of exit in main

ok jung@

> Index: wc.c
> ===
> RCS file: /cvs/src/usr.bin/wc/wc.c,v
> retrieving revision 1.20
> diff -u -p -u -p -r1.20 wc.c
> --- wc.c  8 Dec 2015 01:00:45 -   1.20
> +++ wc.c  8 May 2016 21:51:28 -
> @@ -34,7 +34,6 @@
> #include 
> #include 
> #include 
> -#include 
> #include 
> #include 
> #include 
> @@ -45,12 +44,12 @@
> 
> int64_t   tlinect, twordct, tcharct;
> int   doline, doword, dochar, humanchar, multibyte;
> -int  rval;
> +int  rval;
> extern char *__progname;
> 
> -void print_counts(int64_t, int64_t, int64_t, char *);
> -void format_and_print(long long);
> -void cnt(char *);
> +static void print_counts(int64_t, int64_t, int64_t, char *);
> +static void format_and_print(int64_t);
> +static void cnt(char *);
> 
> int
> main(int argc, char *argv[])
> @@ -82,10 +81,10 @@ main(int argc, char *argv[])
>   break;
>   case '?':
>   default:
> - (void)fprintf(stderr,
> + fprintf(stderr,
>   "usage: %s [-c | -m] [-hlw] [file ...]\n",
>   __progname);
> - exit(1);
> + return 1;
>   }
>   argv += optind;
>   argc -= optind;
> @@ -99,7 +98,7 @@ main(int argc, char *argv[])
>   doline = doword = dochar = 1;
> 
>   if (!*argv) {
> - cnt((char *)NULL);
> + cnt(NULL);
>   } else {
>   int dototal = (argc > 1);
> 
> @@ -111,14 +110,14 @@ main(int argc, char *argv[])
>   print_counts(tlinect, twordct, tcharct, "total");
>   }
> 
> - exit(rval);
> + return rval;
> }
> 
> -void
> +static void
> cnt(char *file)
> {
>   static char *buf;
> - static ssize_t bufsz;
> + static size_t bufsz;
> 
>   FILE *stream;
>   char *C;
> @@ -213,7 +212,7 @@ cnt(char *file)
>   ++charct;
>   len = mbtowc(, C, MB_CUR_MAX);
>   if (len == -1) {
> - (void)mbtowc(NULL, NULL,
> + mbtowc(NULL, NULL,
>   MB_CUR_MAX);
>   len = 1;
>   wc = L' ';
> @@ -263,31 +262,31 @@ cnt(char *file)
>   }
> }
> 
> -void 
> -format_and_print(long long v)
> +static void
> +format_and_print(int64_t v)
> {
>   if (humanchar) {
>   char result[FMT_SCALED_STRSIZE];
> 
> - (void)fmt_scaled(v, result);
> - (void)printf("%7s", result);
> + fmt_scaled((long long)v, result);
> + printf("%7s", result);
>   } else {
> - (void)printf(" %7lld", v);
> + printf(" %7lld", v);
>   }
> }
> 
> -void
> +static void
> print_counts(int64_t lines, int64_t words, int64_t chars, char *name)
> {
>   if (doline)
> - format_and_print((long long)lines);
> + format_and_print(lines);
>   if (doword)
> - format_and_print((long long)words);
> + format_and_print(words);
>   if (dochar)
> - format_and_print((long long)chars);
> + format_and_print(chars);
> 
>   if (name)
> - (void)printf(" %s\n", name);
> + printf(" %s\n", name);
>   else
> - (void)printf("\n");
> + printf("\n");
> }
> 



Re: quiet legacy drivers on amd64

2016-08-02 Thread Joerg Jung

> Am 01.08.2016 um 23:14 schrieb joshua stein :
> 
> are these complaints really helpful on modern machines?

Can't speak for clock. But I tend to not like this for nvram.
IMHO, even with recent HW this is a valid concern and reminder
that someone should handle setting of the nvram options correctly.

> Index: arch/amd64/amd64/nvram.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/nvram.c,v
> retrieving revision 1.6
> diff -u -p -u -p -r1.6 nvram.c
> --- arch/amd64/amd64/nvram.c6 Mar 2016 22:41:24 -1.6
> +++ arch/amd64/amd64/nvram.c1 Aug 2016 21:13:09 -
> @@ -64,8 +64,11 @@ nvramattach(int num)
>printf("nvram: initialized\n");
> #endif
>nvram_initialized = 1;
> -} else
> +}
> +#ifdef NVRAM_DEBUG
> +else
>printf("nvram: invalid checksum\n");
> +#endif
> }
> 
> int
> Index: arch/amd64/isa/clock.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/isa/clock.c,v
> retrieving revision 1.22
> diff -u -p -u -p -r1.22 clock.c
> --- arch/amd64/isa/clock.c14 Mar 2015 03:38:46 -1.22
> +++ arch/amd64/isa/clock.c1 Aug 2016 21:13:09 -
> @@ -428,7 +428,7 @@ clock_expandyear(int clockyear)
>cmoscentury = 0;
>splx(s);
>if (!cmoscentury) {
> -#ifdef DIAGNOSTIC
> +#ifdef DEBUG_CLOCK
>printf("clock: unknown CMOS layout\n");
> #endif
>return (clockyear);
> 



Re: syn cache hash size sysctl

2016-07-19 Thread Joerg Jung


> Am 19.07.2016 um 23:16 schrieb Alexander Bluhm :
> 
>> On Tue, Jul 19, 2016 at 09:48:19PM +0100, Jason McIntyre wrote:
>> oh oh. i should have been clearer: they are sorted in sysctl(3), but in
>> sysctl(8) they are merely listed in the order that running "sysctl"
>> dumps them. so no sort neccessary for sysctl(8).
> 
> So now sysctl(8) has all net.inet.tcp in sysctl output order.
> 
> ok?

ok jung@

> bluhm
> 
> Index: lib/libc/gen/sysctl.3
> ===
> RCS file: /data/mirror/openbsd/cvs/src/lib/libc/gen/sysctl.3,v
> retrieving revision 1.266
> diff -u -p -r1.266 sysctl.3
> --- lib/libc/gen/sysctl.314 Jul 2016 17:34:06 -1.266
> +++ lib/libc/gen/sysctl.319 Jul 2016 20:36:19 -
> @@ -1188,6 +1188,7 @@ The currently defined protocols and name
> .It tcp Ta stats Ta structure Ta no
> .It tcp Ta synbucketlimit Ta integer Ta yes
> .It tcp Ta syncachelimit Ta integer Ta yes
> +.It tcp Ta synhashsize Ta integer Ta yes
> .It tcp Ta synuselimit Ta integer Ta yes
> .It udp Ta baddynamic Ta array Ta yes
> .It udp Ta checksum Ta integer Ta yes
> @@ -1617,6 +1618,10 @@ Returns the TCP statistics in a struct t
> The maximum number of entries allowed per hash bucket in the TCP SYN cache.
> .It Li tcp.syncachelimit
> The maximum number of entries allowed in the TCP SYN cache.
> +.It Li tcp.synhashsize
> +The number of buckets in the TCP SYN cache hash array.
> +After the value is set, the actual size changes when the alternative
> +SYN cache becomes empty and both SYN caches are swapped.
> .It Li tcp.synuselimit
> The minimum number of times the hash function for the TCP SYN cache is used
> before it is reseeded.
> Index: sbin/sysctl/sysctl.8
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sbin/sysctl/sysctl.8,v
> retrieving revision 1.202
> diff -u -p -r1.202 sysctl.8
> --- sbin/sysctl/sysctl.85 Jul 2016 17:41:59 -1.202
> +++ sbin/sysctl/sysctl.819 Jul 2016 21:06:02 -
> @@ -256,7 +256,6 @@ and a few require a kernel compiled with
> .It net.inet.tcp.keepinittime Ta integer Ta yes
> .It net.inet.tcp.keepidle Ta integer Ta yes
> .It net.inet.tcp.keepintvl Ta integer Ta yes
> -.It net.inet.tcp.always_keepalive Ta integer Ta yes
> .It net.inet.tcp.slowhz Ta integer Ta no
> .It net.inet.tcp.baddynamic Ta array Ta yes
> .It net.inet.tcp.sack Ta integer Ta yes
> @@ -266,10 +265,13 @@ and a few require a kernel compiled with
> .It net.inet.tcp.ecn Ta integer Ta yes
> .It net.inet.tcp.syncachelimit Ta integer Ta yes
> .It net.inet.tcp.synbucketlimit Ta integer Ta yes
> -.It net.inet.tcp.synuselimit Ta integer Ta yes
> .It net.inet.tcp.rfc3390 Ta integer Ta yes
> .It net.inet.tcp.reasslimit Ta integer Ta yes
> +.It net.inet.tcp.sackholelimit Ta integer Ta yes
> +.It net.inet.tcp.always_keepalive Ta integer Ta yes
> +.It net.inet.tcp.synuselimit Ta integer Ta yes
> .It net.inet.tcp.rootonly Ta array Ta yes
> +.It net.inet.tcp.synhashsize Ta integer Ta yes
> .It net.inet.udp.checksum Ta integer Ta yes
> .It net.inet.udp.baddynamic Ta array Ta yes
> .It net.inet.udp.recvspace Ta integer Ta yes
> 



Re: syn cache hash size sysctl

2016-07-19 Thread Joerg Jung
On Tue, Jul 19, 2016 at 06:13:42PM +0200, Alexander Bluhm wrote:
> Hi,
> 
> claudio@ suggested to have a tunable size for the syn cache hash
> array.  As we are swapping between two syn caches for random reseeding
> anyway, this feature can be added easily.  When the cache is empty,
> we can change the hash size.
> 
> This allows an admin under SYN flood attack to tune his machine.
> sysctl net.inet.tcp.synhashsize=1

Makes sense to me and I like this.
 
> ok?

ok jung@

Please, also document it, at least in sysctl(8).
 
> bluhm
> 
> Index: netinet/tcp_input.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v
> retrieving revision 1.324
> diff -u -p -r1.324 tcp_input.c
> --- netinet/tcp_input.c   1 Jul 2016 18:37:15 -   1.324
> +++ netinet/tcp_input.c   19 Jul 2016 15:02:35 -
> @@ -3266,7 +3266,7 @@ tcp_mss_adv(struct mbuf *m, int af)
>   */
>  
>  /* syn hash parameters */
> -int  tcp_syn_cache_size = TCP_SYN_HASH_SIZE;
> +int  tcp_syn_hash_size = TCP_SYN_HASH_SIZE;
>  int  tcp_syn_cache_limit = TCP_SYN_HASH_SIZE*TCP_SYN_BUCKET_SIZE;
>  int  tcp_syn_bucket_limit = 3*TCP_SYN_BUCKET_SIZE;
>  int  tcp_syn_use_limit = 10;
> @@ -3360,7 +3360,13 @@ syn_cache_init(void)
>   int i;
>  
>   /* Initialize the hash buckets. */
> - for (i = 0; i < tcp_syn_cache_size; i++) {
> + tcp_syn_cache[0].scs_buckethead = mallocarray(tcp_syn_hash_size,
> + sizeof(struct syn_cache_head), M_SYNCACHE, M_WAITOK|M_ZERO);
> + tcp_syn_cache[1].scs_buckethead = mallocarray(tcp_syn_hash_size,
> + sizeof(struct syn_cache_head), M_SYNCACHE, M_WAITOK|M_ZERO);
> + tcp_syn_cache[0].scs_size = tcp_syn_hash_size;
> + tcp_syn_cache[1].scs_size = tcp_syn_hash_size;
> + for (i = 0; i < tcp_syn_hash_size; i++) {
>   TAILQ_INIT(_syn_cache[0].scs_buckethead[i].sch_bucket);
>   TAILQ_INIT(_syn_cache[1].scs_buckethead[i].sch_bucket);
>   }
> @@ -3377,7 +3383,7 @@ syn_cache_insert(struct syn_cache *sc, s
>   struct syn_cache_set *set = _syn_cache[tcp_syn_cache_active];
>   struct syn_cache_head *scp;
>   struct syn_cache *sc2;
> - int s;
> + int i, s;
>  
>   s = splsoftnet();
>  
> @@ -3385,16 +3391,33 @@ syn_cache_insert(struct syn_cache *sc, s
>* If there are no entries in the hash table, reinitialize
>* the hash secrets.  To avoid useless cache swaps and
>* reinitialization, use it until the limit is reached.
> +  * An emtpy cache is also the oportunity to resize the hash.
>*/
>   if (set->scs_count == 0 && set->scs_use <= 0) {
> - arc4random_buf(set->scs_random, sizeof(set->scs_random));
>   set->scs_use = tcp_syn_use_limit;
> + if (set->scs_size != tcp_syn_hash_size) {
> + scp = mallocarray(tcp_syn_hash_size, sizeof(struct
> + syn_cache_head), M_SYNCACHE, M_NOWAIT|M_ZERO);
> + if (scp == NULL) {
> + /* Try again next time. */
> + set->scs_use = 0;
> + } else {
> + free(set->scs_buckethead, M_SYNCACHE,
> + set->scs_size *
> + sizeof(struct syn_cache_head));
> + set->scs_buckethead = scp;
> + set->scs_size = tcp_syn_hash_size;
> + for (i = 0; i < tcp_syn_hash_size; i++)
> + TAILQ_INIT([i].sch_bucket);
> + }
> + }
> + arc4random_buf(set->scs_random, sizeof(set->scs_random));
>   tcpstat.tcps_sc_seedrandom++;
>   }
>  
>   SYN_HASHALL(sc->sc_hash, >sc_src.sa, >sc_dst.sa,
>   set->scs_random);
> - scp = >scs_buckethead[sc->sc_hash % tcp_syn_cache_size];
> + scp = >scs_buckethead[sc->sc_hash % set->scs_size];
>   sc->sc_buckethead = scp;
>  
>   /*
> @@ -3437,7 +3460,7 @@ syn_cache_insert(struct syn_cache *sc, s
>*/
>   scp2 = scp;
>   if (TAILQ_EMPTY(>sch_bucket)) {
> - sce = >scs_buckethead[tcp_syn_cache_size];
> + sce = >scs_buckethead[set->scs_size];
>   for (++scp2; scp2 != scp; scp2++) {
>   if (scp2 >= sce)
>   scp2 = >scs_buckethead[0];
> @@ -3595,7 +3618,7 @@ syn_cache_lookup(struct sockaddr *src, s
>   if (sets[i]->scs_count == 0)
>   continue;
>   SYN_HASHALL(hash, src, dst, sets[i]->scs_random);
> - scp = [i]->scs_buckethead[hash % tcp_syn_cache_size];
> + scp = [i]->scs_buckethead[hash % sets[i]->scs_size];
>   *headp = scp;
>   TAILQ_FOREACH(sc, >sch_bucket, sc_bucketq) 

Re: cbfb: coreboot framebuffer console

2016-06-13 Thread Joerg Jung

Am 13.06.2016 um 12:35 schrieb Mark Kettenis :

>> Date: Sun, 12 Jun 2016 11:47:37 -0500
>> From: joshua stein 
>> 
>> Here's a new version with feedback from Miod and Ted.
>> 
>> It also fixes a bug on a Broadwell Chromebook (tested by Edward
>> Wandasiewicz) which has proper inteldrm but no vga, so cbfb was
>> getting chosen as the console early on but then not detaching when
>> inteldrm wanted to take over.  This detaches cbfb in the intel and
>> radeon drivers.
> 
> The addition of those hooks makes me even more convinced that efifb(4)
> and cbfb(4) should be the same driver.  The only thing that is really
> different is the data structure that is used to initialize the
> framebuffer parameters.

I tend to agree with Mark, these two should be merged.

> We should not get hung up too much about the driver name.  If it is
> felt that efifb(4) is inappropriate for these chromebooks with
> coreboot, we can always rename the driver.

Yes, renaming makes sense then.

>> Index: dev/wscons/wsconsio.h
>> ===
>> RCS file: /cvs/src/sys/dev/wscons/wsconsio.h,v
>> retrieving revision 1.74
>> diff -u -p -u -p -r1.74 wsconsio.h
>> --- dev/wscons/wsconsio.h30 Mar 2016 23:34:12 -1.74
>> +++ dev/wscons/wsconsio.h12 Jun 2016 16:40:46 -
>> @@ -348,6 +348,7 @@ struct wsmouse_calibcoords {
>> #defineWSDISPLAY_TYPE_INTELDRM69/* Intel KMS framebuffer */
>> #defineWSDISPLAY_TYPE_RADEONDRM 70/* ATI Radeon KMS framebuffer 
>> */
>> #defineWSDISPLAY_TYPE_EFIFB71/* EFI framebuffer */
>> +#defineWSDISPLAY_TYPE_CBFB72/* Coreboot framebuffer */
>> 
>> /* Basic display information.  Not applicable to all display types. */
>> struct wsdisplay_fbinfo {
>> Index: arch/amd64/amd64/cbfb.c
>> ===
>> RCS file: arch/amd64/amd64/cbfb.c
>> diff -N arch/amd64/amd64/cbfb.c
>> --- /dev/null1 Jan 1970 00:00:00 -
>> +++ arch/amd64/amd64/cbfb.c12 Jun 2016 16:40:46 -
>> @@ -0,0 +1,445 @@
>> +/*$OpenBSD$ */
>> +
>> +/*
>> + * Coreboot framebuffer console driver
>> + *
>> + * Copyright (c) 2016 joshua stein 
>> + * Copyright (c) 2015 YASUOKA Masahiko 
>> + *
>> + * Permission to use, copy, modify, and distribute this software for any
>> + * purpose with or without fee is hereby granted, provided that the above
>> + * copyright notice and this permission notice appear in all copies.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
>> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
>> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
>> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
>> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
>> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
>> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>> + */
>> +
>> +#include 
>> +#include 
>> +
>> +#include 
>> +#include 
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
>> +
>> +/* coreboot tables */
>> +
>> +struct cb_header {
>> +union {
>> +uint8_t signature[4]; /* "LBIO" */
>> +uint32_t signature32;
>> +};
>> +uint32_theader_bytes;
>> +uint32_theader_checksum;
>> +uint32_ttable_bytes;
>> +uint32_ttable_checksum;
>> +uint32_ttable_entries;
>> +};
>> +
>> +struct cb_framebuffer {
>> +uint64_tphysical_address;
>> +uint32_tx_resolution;
>> +uint32_ty_resolution;
>> +uint32_tbytes_per_line;
>> +uint8_tbits_per_pixel;
>> +uint8_tred_mask_pos;
>> +uint8_tred_mask_size;
>> +uint8_tgreen_mask_pos;
>> +uint8_tgreen_mask_size;
>> +uint8_tblue_mask_pos;
>> +uint8_tblue_mask_size;
>> +uint8_treserved_mask_pos;
>> +uint8_treserved_mask_size;
>> +};
>> +
>> +struct cb_entry {
>> +uint32_ttag;
>> +#define CB_TAG_VERSION  0x0004
>> +#define CB_TAG_FORWARD  0x0011
>> +#define CB_TAG_FRAMEBUFFER  0x0012
>> +uint32_tsize;
>> +union {
>> +charstring[0];
>> +uint64_t forward;
>> +struct cb_framebuffer fb;
>> +} u;
>> +};
>> +
>> +struct cbfb {
>> +struct rasops_info rinfo;
>> +int depth;
>> +paddr_t paddr;
>> +psize_t psize;
>> +
>> +struct cb_framebuffer table_cbfb;
>> +};
>> +
>> +struct cbfb_softc {
>> +struct device sc_dev;
>> +struct cbfb*sc_fb;
>> +};
>> +
>> +int cbfb_match(struct device *, void *, void *);
>> +void cbfb_attach(struct device *, struct device *, void *);
>> +void cbfb_rasops_preinit(struct cbfb *);
>> 

Re: httpd: fix/style: unbalanced va_start and va_end macros

2016-05-08 Thread Joerg Jung
On Wed, Apr 27, 2016 at 02:43:27PM +0200, Hiltjo Posthuma wrote:
> Hi,
> 
> The following patch for httpd fixes unbalanced va_start() and va_end() macros.
> This is in style with the rest of httpd. Also POSIX says:
> 
> "Each invocation of the va_start() and va_copy() macros shall be matched by a
> corresponding invocation of the va_end() macro in the same function."
> 
> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/stdarg.h.html
> 

Yes agreed, the diff should be committed.
ok jung@ if a dev wants to take care
 
> Index: httpd.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/httpd.c,v
> retrieving revision 1.54
> diff -u -p -r1.54 httpd.c
> --- httpd.c   2 Feb 2016 17:51:11 -   1.54
> +++ httpd.c   27 Apr 2016 12:00:43 -
> @@ -1000,11 +1000,13 @@ kv_set(struct kv *kv, char *fmt, ...)
>   va_list   ap;
>   char*value = NULL;
>   struct kv   *ckv;
> + int ret;
>  
>   va_start(ap, fmt);
> - if (vasprintf(, fmt, ap) == -1)
> - return (-1);
> + ret = vasprintf(, fmt, ap);
>   va_end(ap);
> + if (ret == -1)
> + return (-1);
>  
>   /* Remove all children */
>   while ((ckv = TAILQ_FIRST(>kv_children)) != NULL) {
> @@ -1025,11 +1027,13 @@ kv_setkey(struct kv *kv, char *fmt, ...)
>  {
>   va_list  ap;
>   char*key = NULL;
> + int ret;
>  
>   va_start(ap, fmt);
> - if (vasprintf(, fmt, ap) == -1)
> - return (-1);
> + ret = vasprintf(, fmt, ap);
>   va_end(ap);
> + if (ret == -1)
> + return (-1);
>  
>   free(kv->kv_key);
>   kv->kv_key = key;
> 
> ---
> Kind regards,
> Hiltjo
> 



Re: httpd: patch for portability asprintf use

2016-05-08 Thread Joerg Jung
On Fri, May 06, 2016 at 06:48:38PM +0200, Reyk Floeter wrote:
> 
> > On 06.05.2016, at 18:36, Theo de Raadt  wrote:
> > 
> >> If OpenBSD's behavior of asprintf is non-standard and everyone else is
> >> doing it differently, we would probably have to apply the patch. But this
> >> would also affect many other places in the tree were we rely on our
> >> asprintf semantics.
> > 
> > Actually, we have fixed all usage cases in our tree to be portable.
> > 
> > I have wondered in the past whether we should set the pointer to (void
> > *)-1 instead of NULL, because this NULL return is a trap.
> > 
> 
> 
> I think enforcing it this way would make much sense.
> 
> I'm a candidate for such traps as I only develop C code on/for OpenBSD.
> 
> OK, this makes Hiltjo's diff a valid addition.

yes agreed, please commit it. ok jung@
 
> Reyk
> 



Re: anti-ROP mechanism in libc

2016-04-25 Thread Joerg Jung
On Mon, Apr 25, 2016 at 03:23:47PM +, Robert Peichaer wrote:
> On Mon, Apr 25, 2016 at 10:57:37AM -0400, Ted Unangst wrote:
> > Theo de Raadt wrote:
> > > + cp -p /usr/lib/$_lib /usr/lib/$_tmplib
> > > + install -o root -g bin -m 0444 $_lib /usr/lib/$_lib &&
> > > + rm -f /usr/lib/$_tmplib ||
> > > + mv /usr/lib/$_tmplib /usr/lib/$_lib
> > 
> > I'm a little confused by what's going on here. If the install fails, do we
> > still want to overwrite the lib?
>  
> 
> If the install fails, the original library file is restored.
> 
> The "install .. && rm .. || mv ..." is identical to if-then-else and could
> be written like this too.

Nitpicking: nope, It is not identical, see:
https://github.com/koalaman/shellcheck/wiki/SC2015

Though, may not matter here.

>   if install -o root -g bin -m 0444 $_lib /usr/lib/$_lib; then
>   rm -f /usr/lib/$_tmplib
>   else
>   mv /usr/lib/$_tmplib /usr/lib/$_lib
>   fi
> 



Re: smtpd.conf(5): comma in filter chains

2016-04-10 Thread Joerg Jung

> Am 09.04.2016 um 21:33 schrieb Jason McIntyre :
> 
>> On Sat, Apr 09, 2016 at 11:42:58AM +0200, david+bsd@dahlberg.cologne wrote:
>> After quite some debugging of why the heck my smtpd.conf was not
>> working after upgrading to 5.9 and substituting clamsmtp and dkim-
>> signer by smtpd(8) filters:
>> 
>> smtpd.conf(5) states:
>> ?? filter name chain filter [, ...]
>> but should say:
>> ?? filter name chain filter [...]
>> 
> 
> hi.
> 
> could you explain what works and what doesn;t? from reading this, i'd
> expect the doc is trying to say you can specify one filter like this:
> 
>filter x chain y
> 
> and multiple filters:
> 
>filter x chain y,z
> 
> are you speciying one argument to "chain" or multiple?

If read parse.y correctly filter x chain is allowed to have zero, one, or 
multiple arguments.
Though, only multiple arguments really make sense here.

The given multiple arguments need to be separated by spaces, not commas:

   filter x chain y z

so I think the diff below is right.

> jmc
> 
>> Index: smtpd.conf.5
>> ===
>> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
>> retrieving revision 1.156
>> diff -u -p -r1.156 smtpd.conf.5
>> --- smtpd.conf.57 Mar 2016 16:21:48 -??1.156
>> +++ smtpd.conf.59 Apr 2016 09:32:23 -
>> @@ -612,7 +612,7 @@ using the given filter
>> ??Filters are used to hook into the SMTP dialog and provide additional
>> filtering
>> ??options for
>> ??.Xr smtpd 8 .
>> -.It Ic filter Ar name Ic chain Ar filter Op , Ar ...
>> +.It Ic filter Ar name Ic chain Ar filter Op Ar ...
>> ??Specify a filter chain with the given
>> ??.Ar name
>> ??and filters.
>> 
> 



Re: rcctl ls faulty -> failed

2016-03-29 Thread Joerg Jung
On Tue, Mar 29, 2016 at 08:22:31AM -0600, Todd C. Miller wrote:
> On Tue, 29 Mar 2016 15:29:27 +0200, Antoine Jacoutot wrote:
> 
> > We'd like to rename the 'faulty' listing to 'failed'.
> > i.e. rcctl ls failed
> > 
> > 'faulty' does sound a bit weird and is not obvious to remember.
> > Now the question is should we keep supporting the 'faulty' keyword or not?
> > I'm not in favor of adding a knob especially when it's just an alias; 
> > that'd 
> > also mean documenting it.
> 
> I like this.

Me too.



Re: multitouch support in wsmouse 1/3

2016-03-19 Thread Joerg Jung

> On 17 Mar 2016, at 13:58, Martin Pieuchot  wrote:
> 
> On 13/03/16(Sun) 18:37, Ulf Brosziewski wrote:
>> The diffs below are a rewrite of the input-processing part of wsmouse. It
>> adds support for multitouch input.
>> 
>> I have split the set of diffs into three parts and I will post part 2 and 3
>> in separate messages. Part 1 below contains all patches for wscons, part 2
>> is for the hardware drivers, part 3 is a patch for the synaptics driver in
>> xenocara (compiling that driver will require the modified version of
>> wsconsio.h in /usr/include/dev/wscons).
> 
> I'm really afraid because this is just a single diff split in 3, they
> are not independent.  Plus since your original mails included a broken
> diff I doubt anybody tried it. 
> 
> That said you've done some great work and I like it a lot.
> 
> Could you prepare a diff with all the new stuff that does do change

I guess you wanted to write s/does/doesn’t/ above.
Other than that, yes I agree that would be the right direction.

> anything in the existing code?  This could go in directly.
> 
> Which hardware still need to be tested?
> 



Re: OpenSMTPD and mask-source flag.

2016-02-13 Thread Joerg Jung
On Fri, Feb 12, 2016 at 05:00:59PM -0500, Peter Bisroev wrote:
> > Just in case the previous diff is OK, I am attaching the patch to the
> > smtpd.conf man page.
> 
> Hi Gilles,
> 
> I apologize, my previous manpage diff did not include the information 
> regarding
> the fact that connections through local socket will always be tagged 'local'.
> 
> Please find the corrected manpage diff below.

Some minor comments inline below.
 
> Thank you!
> --peter
> 
> 
> Index: smtpd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
> retrieving revision 1.153
> diff -u -p -r1.153 smtpd.conf.5
> --- smtpd.conf.5  21 Jan 2016 23:40:27 -  1.153
> +++ smtpd.conf.5  12 Feb 2016 21:56:51 -
> @@ -654,6 +654,12 @@ of inflight envelopes falls below
>  Changing the default value might degrade performance.
>  .It Xo
>  .Bk -words
> +.Ic listen on socket
> +.Op Ic filter Ar name
> +.Op Ic mask-source
> +.Ek
> +.br
> +.Bk -words
>  .Ic listen on Ar interface
>  .Op Ar family
>  .Op Ic port Ar port
> @@ -671,11 +677,28 @@ Changing the default value might degrade
>  .Ek
>  .Xc
>  .Pp
> -Specify an
> +Specify a
> +.Ic socket
> +or an
> +.Ar interface
> +to listen on for incoming connections. A

New sentence -> new line, please.

> +.Ic listen on socket
> +directive is used to customize listening behavior for messages submitted
> +through local queues, for example, via the
> +.Xr mail 1
> +utility. This is an optional directive, and if not supplied,

New sentence, new line.

> +.Xr smtpd 8
> +will listen for connections on the
> +.Ic socket
> +only. Clients connecting through the

I do not like the wording "...only." here. Sounds ambiguous. However,
I'm not a native speaker. Also, new sentence, new line.

> +.Ic socket
> +will always be tagged with the 'local'
> +.Ic tag .
> +.Pp
> +To listen on a specific network interface, specify an
>  .Ar interface
> -and
> -.Ar port
> -to listen on.
> +and an optional
> +.Ar port .
>  An interface group, an IP address or a domain name may
>  be used in place of
>  .Ar interface .
> 



Re: can't run multiple instances of httpd, flags not visible in processes

2016-02-01 Thread Joerg Jung
On Mon, Feb 01, 2016 at 07:24:39PM +, Stuart Henderson wrote:
> On 2016/02/01 15:02, Joerg Jung wrote:
> > What about smtpd, should be similar, no?
> 
> This would do the trick. It loses the getrlimit/setrlimit dance that
> config_process() normally does, but I'm not sure if that is really
> needed for the parent process anyway (mine only has 11 FDs so it's
> not in any danger of running out).

I think that is fine and makes sense to me, ok jung@

But please wait for Gilles to comment.
 
> Index: etc/rc.d/smtpd
> ===
> RCS file: /cvs/src/etc/rc.d/smtpd,v
> retrieving revision 1.5
> diff -u -p -r1.5 smtpd
> --- etc/rc.d/smtpd26 Dec 2015 09:55:15 -  1.5
> +++ etc/rc.d/smtpd1 Feb 2016 19:22:33 -
> @@ -6,7 +6,6 @@ daemon="/usr/sbin/smtpd"
>  
>  . /etc/rc.d/rc.subr
>  
> -pexp="smtpd: \[priv\]"
>  rc_reload=NO
>  
>  rc_cmd $1
> Index: usr.sbin/smtpd/smtpd.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.c,v
> retrieving revision 1.272
> diff -u -p -r1.272 smtpd.c
> --- usr.sbin/smtpd/smtpd.c27 Jan 2016 12:46:03 -  1.272
> +++ usr.sbin/smtpd/smtpd.c1 Feb 2016 19:22:33 -
> @@ -667,8 +667,6 @@ main(int argc, char *argv[])
>  
>   fork_peers();
>  
> - config_process(PROC_PARENT);
> -
>   imsg_callback = parent_imsg;
>   event_init();
>  
> 



Re: can't run multiple instances of httpd, flags not visible in processes

2016-02-01 Thread Joerg Jung

> On 01 Feb 2016, at 14:53, Stuart Henderson  wrote:
> 
> moved from misc.
> 
> On 2016-01-28, Antoine Jacoutot  wrote:
>>> Well, we "tradionally" had setproctitle("[priv]") in the parent.  I
>>> changed the tradition to setproctitle("parent").
>>> 
>>> I have no objections with changing this in the parent (but keeping the
>>> setproctitles in the children) to either the default (all command line
>>> flags) or to something like setproctitle("parent, %s", conffile).
>>> Command line flags suck and I don't think that -d or -v would be
>>> helpful in the output, so I prefer the latter.
>> 
>> "-v" is helpful at least for rc.d which needs to match the full args list by 
>> default
>> 
>>> All rc scripts would have to be adjusted by somebody with better rc-fu.
>> 
>> Actually if things are properly done, the non default pexp line in the rc.d 
>> scripts should just be removed and that's it.
>> 
> 
> Here it is for the majority of base daemons. I've left ypldap for now,
> the rc script matches "ldap client" not "parent". radiusd already leaves
> the parent process title as-is (the setproctitle("[main]") are separate
> processes).
> 
> OK?

ok jung@

What about smtpd, should be similar, no?


> Index: etc/rc.d/eigrpd
> ===
> RCS file: /cvs/src/etc/rc.d/eigrpd,v
> retrieving revision 1.2
> diff -u -p -r1.2 eigrpd
> --- etc/rc.d/eigrpd   21 Oct 2015 11:28:02 -  1.2
> +++ etc/rc.d/eigrpd   1 Feb 2016 13:51:58 -
> @@ -6,6 +6,4 @@ daemon="/usr/sbin/eigrpd"
> 
> . /etc/rc.d/rc.subr
> 
> -pexp="eigrpd: parent.*"
> -
> rc_cmd $1
> Index: etc/rc.d/httpd
> ===
> RCS file: /cvs/src/etc/rc.d/httpd,v
> retrieving revision 1.4
> diff -u -p -r1.4 httpd
> --- etc/rc.d/httpd19 Dec 2015 13:45:12 -  1.4
> +++ etc/rc.d/httpd1 Feb 2016 13:51:58 -
> @@ -6,8 +6,6 @@ daemon="/usr/sbin/httpd"
> 
> . /etc/rc.d/rc.subr
> 
> -pexp="httpd: parent.*"
> -
> # child will not return a config parsing error to the parent
> rc_pre() {
>   ${daemon} -n ${daemon_flags}
> Index: etc/rc.d/ldpd
> ===
> RCS file: /cvs/src/etc/rc.d/ldpd,v
> retrieving revision 1.1
> diff -u -p -r1.1 ldpd
> --- etc/rc.d/ldpd 6 Jul 2011 18:55:36 -   1.1
> +++ etc/rc.d/ldpd 1 Feb 2016 13:51:58 -
> @@ -6,6 +6,4 @@ daemon="/usr/sbin/ldpd"
> 
> . /etc/rc.d/rc.subr
> 
> -pexp="ldpd: parent.*"
> -
> rc_cmd $1
> Index: etc/rc.d/npppd
> ===
> RCS file: /cvs/src/etc/rc.d/npppd,v
> retrieving revision 1.1
> diff -u -p -r1.1 npppd
> --- etc/rc.d/npppd20 Sep 2012 12:51:43 -  1.1
> +++ etc/rc.d/npppd1 Feb 2016 13:51:58 -
> @@ -6,6 +6,4 @@ daemon="/usr/sbin/npppd"
> 
> . /etc/rc.d/rc.subr
> 
> -pexp="npppd: main"
> -
> rc_cmd $1
> Index: etc/rc.d/ntpd
> ===
> RCS file: /cvs/src/etc/rc.d/ntpd,v
> retrieving revision 1.2
> diff -u -p -r1.2 ntpd
> --- etc/rc.d/ntpd 14 Sep 2011 02:36:09 -  1.2
> +++ etc/rc.d/ntpd 1 Feb 2016 13:51:58 -
> @@ -6,7 +6,6 @@ daemon="/usr/sbin/ntpd"
> 
> . /etc/rc.d/rc.subr
> 
> -pexp="ntpd: \[priv\]"
> rc_reload=NO
> 
> rc_cmd $1
> Index: etc/rc.d/ospf6d
> ===
> RCS file: /cvs/src/etc/rc.d/ospf6d,v
> retrieving revision 1.1
> diff -u -p -r1.1 ospf6d
> --- etc/rc.d/ospf6d   17 Jul 2011 00:25:58 -  1.1
> +++ etc/rc.d/ospf6d   1 Feb 2016 13:51:58 -
> @@ -6,6 +6,4 @@ daemon="/usr/sbin/ospf6d"
> 
> . /etc/rc.d/rc.subr
> 
> -pexp="ospf6d: parent.*"
> -
> rc_cmd $1
> Index: etc/rc.d/ospfd
> ===
> RCS file: /cvs/src/etc/rc.d/ospfd,v
> retrieving revision 1.1
> diff -u -p -r1.1 ospfd
> --- etc/rc.d/ospfd8 Jul 2011 22:20:07 -   1.1
> +++ etc/rc.d/ospfd1 Feb 2016 13:51:58 -
> @@ -6,6 +6,4 @@ daemon="/usr/sbin/ospfd"
> 
> . /etc/rc.d/rc.subr
> 
> -pexp="ospfd: parent.*"
> -
> rc_cmd $1
> Index: etc/rc.d/relayd
> ===
> RCS file: /cvs/src/etc/rc.d/relayd,v
> retrieving revision 1.2
> diff -u -p -r1.2 relayd
> --- etc/rc.d/relayd   19 Dec 2015 13:45:12 -  1.2
> +++ etc/rc.d/relayd   1 Feb 2016 13:51:58 -
> @@ -6,8 +6,6 @@ daemon="/usr/sbin/relayd"
> 
> . /etc/rc.d/rc.subr
> 
> -pexp="relayd: parent.*"
> -
> # child will not return a config parsing error to the parent
> rc_pre() {
>   ${daemon} -n ${daemon_flags}
> Index: etc/rc.d/ripd
> ===
> RCS file: /cvs/src/etc/rc.d/ripd,v
> retrieving revision 1.1
> diff -u -p -r1.1 ripd
> --- etc/rc.d/ripd 6 Jul 2011 18:55:36 -   1.1
> +++ etc/rc.d/ripd 1 Feb 2016 

Re: [PATCH] uname, arch/machine -> %c, %a update in PKG_PATH

2016-01-31 Thread Joerg Jung
On Mon, Jan 11, 2016 at 03:40:17PM +, Raf Czlonka wrote:
> Hi all,
> 
> Given that PKG_PATH and pkg.conf(5)'s installpath, now supports %c, %a,
> etc. sequences, it might be worth advertising it a bit more by changing
> all relevant uname(1), arch(1)/machine(1) occurrences or (hard-coded
> release versions or hardware architectures for that matter) in the
> documentation.
> 
> While there, I have also taken the liberty of changing ftp.openbsd.org
> to your.local.mirror and ftp to http in packages(7) to keep it
> consistent with other examples.
> 
> Main benefits:
> - as the sequences themselves - not need to hard-code the values
> - no need to run uname, arch/machine is sub-shells any more
> - short and sweet

While I like the reduction below, it seems with upcoming release this
can be further shortened [1].  So might make sense to update the diff 
below?

[1] http://marc.info/?l=openbsd-cvs=145415350609473=2

> Regards,
> 
> Raf
> 
> Index: share/man/man7/packages.7
> ===
> RCS file: /cvs/src/share/man/man7/packages.7,v
> retrieving revision 1.40
> diff -u -p -r1.40 packages.7
> --- share/man/man7/packages.7 24 Oct 2015 08:44:49 -  1.40
> +++ share/man/man7/packages.7 11 Jan 2016 14:26:49 -
> @@ -240,7 +240,7 @@ are supported: pointing
>  .Ev PKG_PATH
>  to a distant package repository, e.g.,
>  .Bd -literal -offset 1n
> -# export PKG_PATH=ftp://ftp.openbsd.org/pub/OpenBSD/5.2/packages/i386/
> +# export PKG_PATH=http://your.local.mirror/pub/OpenBSD/%c/packages/%a/
>  .Ed
>  .Pp
>  will let
> Index: faq/faq15.html
> ===
> RCS file: /cvs/www/faq/faq15.html,v
> retrieving revision 1.116
> diff -u -p -r1.116 faq15.html
> --- faq/faq15.html23 Nov 2015 03:15:50 -  1.116
> +++ faq/faq15.html11 Jan 2016 14:29:33 -
> @@ -203,13 +203,13 @@ A list of possible locations to fetch pa
>  Example 1: fetching from your CD-ROM,
>  assuming you mounted it on /mnt/cdrom
>  
> -$ export PKG_PATH=/mnt/cdrom/$(uname -r)/packages/$(machine -a)/
> +$ export PKG_PATH=/mnt/cdrom/%c/packages/%a/
>  
>  
>  
>  Example 2: fetching from a nearby mirror
>  
> -$ export PKG_PATH=http://your.local.mirror/pub/OpenBSD/$(uname 
> -r)/packages/$(machine -a)/
> +$ export PKG_PATH=http://your.local.mirror/pub/OpenBSD/%c/packages/%a/
>  
>  
>  
> @@ -404,7 +404,7 @@ HTTP, or SCP locations.
>  Let's consider installation via HTTP in the next example:
>  
>  
> -# pkg_add http://your.local.mirror/pub/OpenBSD/$(uname 
> -r)/packages/$(machine -a)/screen-4.0.3p3.tgz
> +# pkg_add 
> http://your.local.mirror/pub/OpenBSD/%c/packages/%a/screen-4.0.3p3.tgz
>  screen-4.0.3p3: complete
>  
>  
> Index: faq/faq9.html
> ===
> RCS file: /cvs/www/faq/faq9.html,v
> retrieving revision 1.116
> diff -u -p -r1.116 faq9.html
> --- faq/faq9.html 23 Nov 2015 03:16:31 -  1.116
> +++ faq/faq9.html 11 Jan 2016 14:29:33 -
> @@ -362,7 +362,7 @@ To find out more about the packages and 
>  To install the above mentioned package you would issue
>  
>  
> -# export PKG_PATH=http://your.local.mirror/pub/OpenBSD/$(uname 
> -r)/packages/$(uname -m)/
> +# export PKG_PATH=http://your.local.mirror/pub/OpenBSD/%c/packages/%a/
>  # pkg_add fedora_base
>  
>  
> Index: faq/pf/example1.html
> ===
> RCS file: /cvs/www/faq/pf/example1.html,v
> retrieving revision 1.63
> diff -u -p -r1.63 example1.html
> --- faq/pf/example1.html  10 Jan 2016 01:28:23 -  1.63
> +++ faq/pf/example1.html  11 Jan 2016 14:29:33 -
> @@ -346,7 +346,7 @@ to use (8.8.8.8@53, for example
>  installing the tool and choosing a resolver.
>  
>  
> -# export PKG_PATH=http://ftp.openbsd.org/pub/OpenBSD/$(uname 
> -r)/packages/$(uname -m)
> +# export PKG_PATH=http://your.local.mirror/pub/OpenBSD/%c/packages/%a/
>  # pkg_add dnscrypt-proxy
>  # rcctl enable dnscrypt_proxy
>  
> Index: faq/ports/guide.html
> ===
> RCS file: /cvs/www/faq/ports/guide.html,v
> retrieving revision 1.46
> diff -u -p -r1.46 guide.html
> --- faq/ports/guide.html  21 Dec 2015 16:35:48 -  1.46
> +++ faq/ports/guide.html  11 Jan 2016 14:29:33 -
> @@ -563,7 +563,7 @@ When dealing with multi-packages, it may
>  >pkg_add(1) and
>   href="http://www.openbsd.org/cgi-bin/man.cgi?sektion=1query=pkg_delete;
>  >pkg_delete(1) directly,
> -setting PKG_PATH to /usr/ports/packages/`arch -s`/all/ in 
> the
> +setting PKG_PATH to /usr/ports/packages/%a/all/ in the
>  environment.
>  
>  
> 



Re: [diff] lpbl(4): driver for TI LP8550 backlight controller (found in e.g. MacBook Air 6,2)

2016-01-30 Thread Joerg Jung
On Wed, Jan 27, 2016 at 05:20:59AM +0200, Sviatoslav Chagaev wrote:
> On Sun, 10 Jan 2016 21:28:31 +0100 Joerg Jung wrote:
> > On Sun, Jan 10, 2016 at 04:59:22AM +0200, Sviatoslav Chagaev wrote: 
> > > So below is a driver for TI LP8550.
> ...
> > > If there's any chance for it to be commited, please tell me what I need to
> > > fix/improve to get it commited (so I don't have to reapply it every time I
> > > upgrade).
> > 
> > IMHO, instead of adding another driver to the mix, I would prefer this 
> > to be handled through the associated asmc(4) keys instead of accessing
> > the chip directly.  The SMC is supposed to be the central point for such
> > manipulations.  Unfortunately, the keys are not documented and need some
> > non-trivial effort to be figured out.
> > 
> > If this is not possible through asmc(4) and if your new driver improves
> > the situation then I'm fine with importing it, but for now it is just
> > worse.
> > 
> 
> Using knowledge acquired from [1]

[1] contains some "wrong" (obsolete?) information, e.g. the Errors list.

> and [2] made a diff, posted below for
> reference, which adds:
>  * ioctl interface to asmc(4) to allow reading/writing of ``keys'' from
>userspace;
>  * companion smcprobe(1) which reads/writes ``keys'' using the ioctl;

Wow..., much effort for simple key dumping.  Please, find below an
(older/may not apply) diff which just dumps the keys directly from
kernel into dmesg.

Are you aware that keys can be all kind of types, e.g. structs?  Have
you seen the ASMC_INFO (0x13) command which might be used to determine
the type?  The key flags (read/write/atomic...) are mostly documented
somewhere.

>  * smcdumpkeys(1): scans an SMC textual firmware file and dumps the ``key''
>table that it contains.

Are you aware of the ASMC_INDEX (0x12) command, which can easily be used
to just iterate ALL keys based on index?

> Downloaded SMC FW [3], unpacked it using Google Drive and used smcdumpkeys(1) 
> +
> smcprobe(1) to create the table of keys and their current values before 
> suspend
> [4] and after wake up [5]. Diffed them [6], 

The diff approach is interesting... :) From your diff output I would try
to play with IPBF, MSXk, SAPN, and DM0T.

> filtering out keys which change over
> time as they are probably sensors (some might have gotten through).

If they start with:

'A'... usually ambient light sensor related,
'F'... fan sensors,
'T'... temperature sensors,
'P'... power related,
'V'... voltage sensors.

> Tried modifying all keys whose names start with 'B': no effect.

'B'... is likely not "backlight" but "battery".

> Tried modifying keys which are different after wake up: no effect.

Please find a more exhaustive/descriptive list of keys here:
http://web.archive.org/web/20151103144947/http://www.parhelia.ch/blog/statics/k3_keys.html

Have you seen this: https://github.com/gcsgithub/smc_util it contains
more details for the various structs.

You may also want to have a look into the existing FreeBSD and Linux
drivers.

> [1] 
> https://reverse.put.as/wp-content/uploads/2015/12/D1_02_Alex_Ninjas_and_Harry_Potter.pdf
> [2] 
> https://dev.inversepath.com/download/public/embedded_systems_exploitation.pdf
> [3] https://support.apple.com/kb/DL1748?locale=en_US
> [4] https://gist.github.com/S010/26145f4446fcc0b8a0d6#file-before_suspend-txt
> [5] https://gist.github.com/S010/26145f4446fcc0b8a0d6#file-after_wakeup-txt
> [6] 
> https://gist.github.com/S010/26145f4446fcc0b8a0d6#file-suspend_wakeup_key-diff


Index: asmc.c
===
RCS file: /cvs/src/sys/dev/isa/asmc.c,v
retrieving revision 1.17
diff -u -p -r1.17 asmc.c
--- asmc.c  12 Dec 2015 12:34:05 -  1.17
+++ asmc.c  12 Dec 2015 13:30:12 -
@@ -32,6 +32,8 @@
 #include 
 #include 
 
+#define ASMC_DEBUG 1
+
 #define ASMC_BASE  0x300   /* SMC base address */
 #define ASMC_IOSIZE32  /* I/O region size 0x300-0x31f */
 
@@ -42,6 +44,7 @@
 
 #define ASMC_READ  0x10/* SMC read command */
 #define ASMC_WRITE 0x11/* SMC write command */
+#define ASMC_INDEX 0x12/* SMC index command */
 #define ASMC_INFO  0x13/* SMC info/type command */
 
 #define ASMC_RETRY 10
@@ -85,6 +88,9 @@ struct asmc_softc {
 };
 
 intasmc_try(struct asmc_softc *, int, const char *, uint8_t *, uint8_t);
+#ifdef ASMC_DEBUG
+void   asmc_dump(struct asmc_softc *, uint32_t);
+#endif
 
 void   asmc_init(void *);
 void   asmc_refresh(void *);
@@ -292,7 +298,9 @@ asmc_attach(struct device *parent, struc
}
printf(", %u key%s\n", ntohl(*(uint32_t *)buf),
(ntohl(*(uint32_t *)buf) == 1) ? "" : "s");
-
+#ifdef ASMC_DEBUG
+   asmc_dump(sc, n

Re: Use HTML entities when referring to tables in www/opensmtpd/faq/example1.html

2016-01-24 Thread Joerg Jung
On Sat, Jan 23, 2016 at 10:21:38PM +, Michael Savage wrote:
> The page at http://www.openbsd.org/opensmtpd/faq/example1.html doesn't
> display correctly because browsers try to interpret /
> as HTML tags. This patch replaces < and > with  and .

This fix was committed by TJ.  I also fixed two more instances.

Thanks for reporting.
 
> Index: example1.html
> ===
> RCS file: /cvs/www/opensmtpd/faq/example1.html,v
> retrieving revision 1.1
> diff -u -p -r1.1 example1.html
> --- example1.html   22 Jan 2016 19:58:33 -  1.1
> +++ example1.html   23 Jan 2016 22:17:05 -
> @@ -146,9 +146,9 @@ listen on all port 587 filter sub tls-re
>  #limit mta for domain gmail.com inet4
> 
>  # allow local messages
> -accept from local for local alias  deliver to lmtp
> "/var/dovecot/lmtp" rcpt-to
> +accept from local for local alias aliases deliver to lmtp
> "/var/dovecot/lmtp" rcpt-to
>  # allow virtual domains
> -accept from any for domain  virtual  deliver to lmtp
> "/var/dovecot/lmtp" rcpt-to
> +accept from any for domain domains virtual virtuals
> deliver to lmtp "/var/dovecot/lmtp" rcpt-to
>  # allow outgoing mails
>  accept from local for any relay
>  
> 



Re: security(8) mailbox check question

2016-01-23 Thread Joerg Jung
On Sat, Jan 23, 2016 at 08:31:09PM +0100, Ingo Schwarze wrote:
> Hi,
> 
> the smtpd(8) daemon supports "deliver to maildir" out of the box,
> and even though putting the user maildirs below /var/mail/ is not
> the default, it's one of many possible and logical choices, and i
> see nothing wrong with it.

This was discussed several times before.  The last one I remember
is [1]. 
 
> Adam Wolk noticed on misc@ that currently security(8) doesn't
> like that choice.  I consider the complaint gratuitious and the
> code to prevent it simple enough that i'd like to commit it.

Personally, I think it makes sense to stay restrictive with the check in
security and I moved away my maildirs from /var/mail years ago for this
reason.  Maybe it should be documented in hier(7) that only mbox is
intended...

[1] http://marc.info/?l=openbsd-misc=133460345221298=2

 
> Any OKs?
>   Ingo
> 
> 
> Index: security
> ===
> RCS file: /cvs/src/libexec/security/security,v
> retrieving revision 1.36
> diff -u -p -r1.36 security
> --- security  21 Jul 2015 19:07:13 -  1.36
> +++ security  23 Jan 2016 19:09:21 -
> @@ -449,7 +449,7 @@ sub check_dot_writeable {
>   }
>  }
>  
> -# Mailboxes should be owned by the user and unreadable.
> +# Mailboxes should be owned by the user, and readable by the user only.
>  sub check_mailboxes {
>   my $dir = '/var/mail';
>   nag !(opendir my $dh, $dir), "opendir: $dir: $!" and return;
> @@ -464,7 +464,9 @@ sub check_mailboxes {
>   my $gname = (getgrgid $fgid)[0] // $fgid;
>   nag $fname ne $name,
>   "user $name mailbox is owned by $fname";
> - nag S_IMODE($mode) != (S_IRUSR | S_IWUSR),
> + my $wantmode = S_IRUSR | S_IWUSR;
> + $wantmode |= S_IXUSER if -d "$dir/$name";
> + nag S_IMODE($mode) != $wantmode,
>   sprintf 'user %s mailbox is %s, group %s',
>   $name, strmode($mode), $gname;
>   }
> 



Re: [diff] lpbl(4): driver for TI LP8550 backlight controller (found in e.g. MacBook Air 6,2)

2016-01-10 Thread Joerg Jung
On Sun, Jan 10, 2016 at 04:59:22AM +0200, Sviatoslav Chagaev wrote:
> Hi,
> 
> I'm running -current on Apple MacBook Air 6,2. I installed using Jasper's
> instructions [1], OpenBSD is the only OS and boots via EFI.
> 
> I'm experiencing a problem with LCD backlight: on wakeup from suspend, the
> backlight brightness is not restored and becomes unadjustable. If adjusted 
> down,
> it turns off, if adjusted up, it goes to 100%.

I also own a MacBook Air 6,2 and wakeup does not really work at all for
me, as the screen just stays black.  Basically I see the same as
reported in [5] for the MacBook Pro.
 
> An Intel developer claims [2] that it's not a bug in Intel KMS driver and 
> people
> claim [3] the problem goes away when booting in legacy BIOS mode.

That makes no sense to me.  If the problem goes aways when booting
legacy BIOS, so it must be a bug when not, right?

> It turns out [4], a numbers of MacBook models [5], including mine, have a 
> Texas
> Instruments LP8550 LED backlight controller on I2C bus. By default, it's
> controlled by external PWM. But it can be configured to be controlled by it's
> own internal register instead.
> 
> So below is a driver for TI LP8550.
> 
> With this driver, after wake up from suspend, the brightness level is restored
> and can be freely adjusted.

I've tested the driver and while it compiles and attaches fine it does
not work for me.  After switching to console the screen stays black.

> The catch is that Intel KMS driver seems to control the chip's ``EN'' pin
> (basically an on/off pin). So whenever Intel KMS toggles the backlight, namely
> on power management (e.g. `xset dpms force off`) and on Xorg <--> VT switch, 
> the chip's brightness control register gets reset and you need to 
> `wsconsctl display.backlight=` to turn the backlight back on.
> 
> But it beats rebooting.

Not for me, with my machine -current works better without your patch,
because I can switch from X to console and back and brightness is
correctly adjusted.

> If there's any chance for it to be commited, please tell me what I need to
> fix/improve to get it commited (so I don't have to reapply it every time I
> upgrade).

IMHO, instead of adding another driver to the mix, I would prefer this 
to be handled through the associated asmc(4) keys instead of accessing
the chip directly.  The SMC is supposed to be the central point for such
manipulations.  Unfortunately, the keys are not documented and need some
non-trivial effort to be figured out.

If this is not possible through asmc(4) and if your new driver improves
the situation then I'm fine with importing it, but for now it is just
worse.

> Next step could be to somehow integrate it with Intel KMS.

Yes, for me this is must-have before it can be committed.

Some minor style comments on the code inline below.

> [1] https://blog.jasper.la/openbsd-uefi-bootloader-howto/
> [2] https://bugs.freedesktop.org/show_bug.cgi?id=67454#c103
> [3] https://bugs.freedesktop.org/show_bug.cgi?id=67454#c58
> [4] https://bugs.freedesktop.org/show_bug.cgi?id=67454#c78
> [5] http://marc.info/?l=openbsd-misc=144988400031788=2
> 
> 
> 
> Index: share/man/man4/iic.4
> ===
> RCS file: /cvs/src/share/man/man4/iic.4,v
> retrieving revision 1.84
> diff -u -p -u -p -r1.84 iic.4
> --- share/man/man4/iic.4  27 Dec 2014 16:08:03 -  1.84
> +++ share/man/man4/iic.4  10 Jan 2016 01:52:55 -
> @@ -153,6 +153,8 @@ National Semiconductor LM87 temperature,
>  National Semiconductor LM93 temperature, voltage, and fan sensor
>  .It Xr lmtemp 4
>  National Semiconductor LM75/LM76/LM77 temperature sensor
> +.It Xr lpbl 4
> +Texas Instruments LP8550 LED backlight controller
>  .It Xr maxds 4
>  Maxim DS1624/DS1631/DS1721 temperature sensor
>  .It Xr maxtmp 4
> Index: share/man/man4/lpbl.4
> ===
> RCS file: share/man/man4/lpbl.4
> diff -N share/man/man4/lpbl.4
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ share/man/man4/lpbl.4 10 Jan 2016 01:52:55 -
> @@ -0,0 +1,48 @@
> +.\"
> +.\" Copyright (c) 2016 Sviatoslav Chagaev 
> +.\"
> +.\" Permission to use, copy, modify, and distribute this software for any
> +.\" purpose with or without fee is hereby granted, provided that the above
> +.\" copyright notice and this permission notice appear in all copies.
> +.\"
> +.\" THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> +.\" WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> +.\" MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> +.\" ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> +.\" WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> +.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> +.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> +.\"
> +.Dd 

Re: ifconfig: rm not need variable noprint

2016-01-01 Thread Joerg Jung
On Wed, Dec 30, 2015 at 05:24:27PM +0100, Fabian Raetz wrote:
> Hi tech@,
> 
> this patch removes the 'noprint' variable which was added to ifconfig.c in 
> rev 1.216
> and is not in use since rev. 1.220.

Committed, thanks!
 
> Cheers,
> Fabian
> 
> 
> Index: sbin/ifconfig/ifconfig.c
> ===
> --- sbin/ifconfig/ifconfig.c.orig
> +++ sbin/ifconfig/ifconfig.c
> @@ -604,7 +604,6 @@ main(int argc, char *argv[])
>   int Cflag = 0;
>   int gflag = 0;
>   int i;
> - int noprint = 0;
>  
>   /* If no args at all, print all interfaces.  */
>   if (argc < 2) {
> @@ -760,7 +759,7 @@ nextarg:
>   argc--, argv++;
>   }
>  
> - if (argc == 0 && actions == 0 && !noprint) {
> + if (argc == 0 && actions == 0) {
>   printif(ifr.ifr_name, aflag ? ifaliases : 1);
>   exit(0);
>   }
> 



Re: asmc(4) improvements

2015-12-22 Thread Joerg Jung
On Mon, Dec 21, 2015 at 10:38:24PM +0100, Mark Kettenis wrote:
> The asmc(4) driver is a bit slow.  On my MacBookPro12,1 it takes a
> couple of minutes to probe all the sensors.  And changing the keyboard
> backlight takes a couple of seconds.  This is especially annoying when
> you're writing a diff to bind that functionality to the keys reserved
> for this purpose on the keyboard.
> 
> Turns out the chip used for the Apple SMC is my old friend the H8S
> microcontroller (see lom(4) on sparc64).  With that knowledge in hand,
> it was fairly obvious what was going wrong.  The driver didn't check
> whether the chip input buffer was full before writing to it.  That
> caused commands to get lost.  With that fixed, comunication with the
> chip seems to be reliable, but still somewhat slow.  

Now, that really is an improvement. Thanks for looking into this and the
earlier bug fix x-mas present!

> There were two reasons for this slowness:
> 
> 1. The asmc_command() function does additional reads as a "sanity
>flush".  We had to wait for these reads to time out, which took a
>significant fraction of a second.  On my system, I neversaw any of
>these flusing reads succeed.  So I removed that bit of code.

I added this sanity flush, because Apple seems to keep changing the
returned number of bytes for some keys depending on the firmware
revision.  An example for are the light sensors keys "ALVn": on newer
Macbooks they return 10 bytes, while on older they return only 6 Bytes.
The reason is that newer Macbooks provide the Lux value directly
(instead of ADC raw data) in the added four bytes (see asmc_light).

So from my understanding: if the buffer is not emptied/flushed you might
read the data from previous command with the next key? Maybe I got that
wrong?

> 2. Failing commands were retried up to 10 times.  I assume this was
>necessary because writing to the chip was unreliable.  

Yes.

>I reduce it to 3 retries.

Makes sense with your buf fix.
 
> 3. The timouts for individual reads and writes were quite high.
>Especially because the code used tsleep(), which means a context
>switch could happen and we'd sleep longer than intended.

Yes.

> Since I also noticed that the command to set the keyboard backlight
> succeeded initially (when a simple delay was used) but failed later
> (when tsleep was used), I concluded that the SMC times out as well if
> we didn't send the complete command within a certain time.  

I think this conclusion is right.  That is what I have seen in my tests
as well.

> Such a minimum time is hard to guarantee if we use tsleep.  So this
> driverreally has to use an actual (spinning) delay, just like what I
> did in my lom(4) driver.
> 
> That sounds worse than it is.  Typically we spin only for a short time
> (500 us seems to be typical) for read and write operations that
> succeed.  So I clamped it at 5 ms.  With the diff below, the asmc
> taskq consumes about the same amount of CPU as the acpi thread.  I
> think that is perfectly acceptable.

Agreed, that totally makes sense. I just added the tsleep() delays to
make things working... now, with your input buffer fix they are not
really needed anymore.
 
> However, since Apple's SMC implementation almost certainly changed
> over the years, it would be good to test this diff on a wider range of
> Apple hardware.  Please check that all sensors are still present and
> are properly updated after applying this diff.

Works fine here.

ok jung@  (but we really should discuss the sanity flush removal)
 
> Thanks,
> 
> Mark
> 
> 
> Index: asmc.c
> ===
> RCS file: /cvs/src/sys/dev/isa/asmc.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 asmc.c
> --- asmc.c15 Dec 2015 20:58:22 -  1.21
> +++ asmc.c21 Dec 2015 21:35:05 -
> @@ -44,7 +44,11 @@
>  #define ASMC_WRITE   0x11/* SMC write command */
>  #define ASMC_INFO0x13/* SMC info/type command */
>  
> -#define ASMC_RETRY   10
> +#define ASMC_OBF 0x01/* Output buffer full */
> +#define ASMC_IBF 0x02/* Input buffer full */
> +#define ASMC_ACCEPT  0x04
> +
> +#define ASMC_RETRY   3
>  #define ASMC_MAXLEN  32  /* SMC maximum data size len */
>  #define ASMC_NOTFOUND0x84/* SMC status key not found */
>  
> @@ -385,11 +389,8 @@ asmc_kbdled(void *arg)
>   int r;
>  
>   if ((r = asmc_try(sc, ASMC_WRITE, "LKSB", buf, 2)))
> -#if 0 /* todo: write error, as no 0x04 wait status, but led is changed */
>   printf("%s: keyboard backlight failed (0x%x)\n",
>   sc->sc_dev.dv_xname, r);
> -#endif
> - ;
>  }
>  
>  int
> @@ -436,44 +437,57 @@ asmc_status(struct asmc_softc *sc)
>  }
>  
>  static int
> -asmc_wait(struct asmc_softc *sc, uint8_t m)
> +asmc_write(struct asmc_softc *sc, uint8_t off, uint8_t val)
>  {
> - int us;
> + uint8_t str;
> + int i;
>  
> - for (us = (2 << 4); us < (2 << 16); us *= 

Re: preparing multitouch support - request for tests

2015-12-16 Thread Joerg Jung
On Wed, Dec 16, 2015 at 02:32:44AM +0100, Ulf Brosziewski wrote:
> Ping? No further thoughts on this, no tests? Do I have to conclude that
> most people are happy with wsmouse as it is?

Yes and no.  As I told you earlier, I think your diffs contain some very
good work and are required for further improvements and progress.  The
question is how to get them integrated.  Further isolated work is
probably the wrong way.  You should try to get things (in small pieces)
into the tree, though likely to late in current cycle. 
 
> I'm aware that the diff isn't small, but anything smaller would make
> some of the remaining parts void and possibly blur the concept.

It depends, for example you can commit the new file additions separately
without hooking them, giving people a chance to view and play with them
in tree.  

> ... 
> 
> There is a second point concerning compat mode that I would like to
> change: it could be made useful. Because of the arbitrary scaling and
> the unpredictable pointer movement, I cannot use it with wsmoused at the
> console. Do touchpads exist where it works?  Recently Thierry Deval
> posted a diff here which proved that we could easily do something about
> that, but that is a different story. In my diff, wsmouseinput hooks its
> "touchpad extension" (wstpad) into the compat-mode conversion function,
> which works well with ws for all touchpads that are available to me.

I tend to agree that this is probably the right direction.



Re: patch 1/4 add generic keyboard backlight support

2015-12-11 Thread Joerg Jung
Ping. Anyone?

> On 07 Dec 2015, at 23:39, Joerg Jung <m...@umaxx.net> wrote:
> 
> Hi,
> 
> here comes a series of small diffs which add generic support for
> keyboard backlights.
> 
> Please find below the first diff, which adds new ioctls to wskbd(4) to
> control keyboard backlights.
> 
> In contrast to an earlier diff from jcs, I have chosen to use a struct
> in favor of a simple (unsigned) int as depending on the vendor
> (Thinkpad, Apple, ...) different min/max values for the brightness of
> the keyboard backlight are possible.
> 
> Comments, OK?
> 
> Thanks,
> Regards,
> Joerg
> 
> 
> 
> Index: wsconsio.h
> ===
> RCS file: /cvs/src/sys/dev/wscons/wsconsio.h,v
> retrieving revision 1.72
> diff -u -p -r1.72 wsconsio.h
> --- wsconsio.h30 Aug 2015 10:05:09 -  1.72
> +++ wsconsio.h7 Dec 2015 21:03:45 -
> @@ -180,6 +180,13 @@ struct wskbd_map_data {
> #define WSKBDIO_GETENCODING   _IOR('W', 15, kbd_t)
> #define WSKBDIO_SETENCODING   _IOW('W', 16, kbd_t)
> 
> +/* Get/set keyboard backlight.  Not applicable to all keyboard types. */
> +struct wskbd_backlight {
> + unsigned int min, max, curval;
> +};
> +#define  WSKBDIO_GETBACKLIGHT_IOR('W', 17, struct wskbd_backlight)
> +#define  WSKBDIO_SETBACKLIGHT_IOW('W', 18, struct wskbd_backlight)
> +
> /* internal use only */
> #define WSKBDIO_SETMODE   _IOW('W', 19, int)
> #define WSKBDIO_GETMODE   _IOR('W', 20, int)
> Index: wskbd.c
> ===
> RCS file: /cvs/src/sys/dev/wscons/wskbd.c,v
> retrieving revision 1.82
> diff -u -p -r1.82 wskbd.c
> --- wskbd.c   10 Sep 2015 18:14:52 -  1.82
> +++ wskbd.c   7 Dec 2015 21:03:45 -
> @@ -230,6 +230,9 @@ int   wskbd_mux_close(struct wsevsrc *);
> int   wskbd_do_open(struct wskbd_softc *, struct wseventvar *);
> int   wskbd_do_ioctl(struct device *, u_long, caddr_t, int, struct proc *);
> 
> +int  (*wskbd_get_backlight)(struct wskbd_backlight *);
> +int  (*wskbd_set_backlight)(struct wskbd_backlight *);
> +
> struct cfdriver wskbd_cd = {
>   NULL, "wskbd", DV_TTY
> };
> @@ -1010,6 +1013,7 @@ wskbd_displayioctl(struct device *dev, u
>   case WSKBDIO_SETDEFAULTKEYREPEAT:
>   case WSKBDIO_SETMAP:
>   case WSKBDIO_SETENCODING:
> + case WSKBDIO_SETBACKLIGHT:
>   if ((flag & FWRITE) == 0)
>   return (EACCES);
>   }
> @@ -1155,6 +1159,18 @@ getkeyrepeat:
>   wsmux_set_layout(sc->sc_base.me_parent, enc);
> #endif
>   return (0);
> +
> + case WSKBDIO_GETBACKLIGHT:
> + if (wskbd_get_backlight != NULL)
> + return (*wskbd_get_backlight)((struct wskbd_backlight 
> *)data);
> + error = ENOTTY;
> + break;
> +
> + case WSKBDIO_SETBACKLIGHT:
> + if (wskbd_set_backlight != NULL)
> + return (*wskbd_set_backlight)((struct wskbd_backlight 
> *)data);
> + error = ENOTTY;
> + break;
>   }
> 
>   /*
> 



Re: patch 1/4 add generic keyboard backlight support

2015-12-11 Thread Joerg Jung

> Am 11.12.2015 um 10:27 schrieb Ville Valkonen <weezeld...@gmail.com>:
> 
>> On 11 December 2015 at 10:36, Joerg Jung <m...@umaxx.net> wrote:
>> Ping. Anyone?
>> 
>> > On 07 Dec 2015, at 23:39, Joerg Jung <m...@umaxx.net> wrote:
>> >
>> > Hi,
>> >
>> > here comes a series of small diffs which add generic support for
>> > keyboard backlights.
>> >
>> > Please find below the first diff, which adds new ioctls to wskbd(4) to
>> > control keyboard backlights.
>> >
>> > In contrast to an earlier diff from jcs, I have chosen to use a struct
>> > in favor of a simple (unsigned) int as depending on the vendor
>> > (Thinkpad, Apple, ...) different min/max values for the brightness of
>> > the keyboard backlight are possible.
>> >
>> > Comments, OK?
>> >
>> > Thanks,
>> > Regards,
>> > Joerg
>> >
>> >
>> >
>> > Index: wsconsio.h
>> > ===
>> > RCS file: /cvs/src/sys/dev/wscons/wsconsio.h,v
>> > retrieving revision 1.72
>> > diff -u -p -r1.72 wsconsio.h
>> > --- wsconsio.h30 Aug 2015 10:05:09 -  1.72
>> > +++ wsconsio.h7 Dec 2015 21:03:45 -
>> > @@ -180,6 +180,13 @@ struct wskbd_map_data {
>> > #define WSKBDIO_GETENCODING   _IOR('W', 15, kbd_t)
>> > #define WSKBDIO_SETENCODING   _IOW('W', 16, kbd_t)
>> >
>> > +/* Get/set keyboard backlight.  Not applicable to all keyboard types. */
>> > +struct wskbd_backlight {
>> > + unsigned int min, max, curval;
>> > +};
>> > +#define  WSKBDIO_GETBACKLIGHT_IOR('W', 17, struct wskbd_backlight)
>> > +#define  WSKBDIO_SETBACKLIGHT_IOW('W', 18, struct wskbd_backlight)
>> > +
>> > /* internal use only */
>> > #define WSKBDIO_SETMODE   _IOW('W', 19, int)
>> > #define WSKBDIO_GETMODE   _IOR('W', 20, int)
>> > Index: wskbd.c
>> > ===
>> > RCS file: /cvs/src/sys/dev/wscons/wskbd.c,v
>> > retrieving revision 1.82
>> > diff -u -p -r1.82 wskbd.c
>> > --- wskbd.c   10 Sep 2015 18:14:52 -  1.82
>> > +++ wskbd.c   7 Dec 2015 21:03:45 -
>> > @@ -230,6 +230,9 @@ int   wskbd_mux_close(struct wsevsrc *);
>> > int   wskbd_do_open(struct wskbd_softc *, struct wseventvar *);
>> > int   wskbd_do_ioctl(struct device *, u_long, caddr_t, int, struct proc *);
>> >
>> > +int  (*wskbd_get_backlight)(struct wskbd_backlight *);
>> > +int  (*wskbd_set_backlight)(struct wskbd_backlight *);
>> > +
>> > struct cfdriver wskbd_cd = {
>> >   NULL, "wskbd", DV_TTY
>> > };
>> > @@ -1010,6 +1013,7 @@ wskbd_displayioctl(struct device *dev, u
>> >   case WSKBDIO_SETDEFAULTKEYREPEAT:
>> >   case WSKBDIO_SETMAP:
>> >   case WSKBDIO_SETENCODING:
>> > + case WSKBDIO_SETBACKLIGHT:
>> >   if ((flag & FWRITE) == 0)
>> >   return (EACCES);
>> >   }
>> > @@ -1155,6 +1159,18 @@ getkeyrepeat:
>> >   wsmux_set_layout(sc->sc_base.me_parent, enc);
>> > #endif
>> >   return (0);
>> > +
>> > + case WSKBDIO_GETBACKLIGHT:
>> > + if (wskbd_get_backlight != NULL)
>> > + return (*wskbd_get_backlight)((struct 
>> > wskbd_backlight *)data);
>> > + error = ENOTTY;
>> > + break;
>> > +
>> > + case WSKBDIO_SETBACKLIGHT:
>> > + if (wskbd_set_backlight != NULL)
>> > + return (*wskbd_set_backlight)((struct 
>> > wskbd_backlight *)data);
>> > + error = ENOTTY;
>> > + break;
>> >   }
>> >
>> >   /*
>> >
> 
> not sure if this diff has been applied in the snapshots but lately brightness 
> keys started to work on Lenovo X250. Could be completely unrelated, though.

Yes, this is unrelated.

Re: [patch] mailwrapper: remove broken fallback code

2015-12-08 Thread Joerg Jung

> On 08 Dec 2015, at 10:39, Sunil Nimmagadda  wrote:
> 
>> If /etc/mailer.conf doesn't exist, mailwrapper tries to run sendmail,
>> giving a confusing error message:
>> 
>> mailwrapper: cannot exec /usr/libexec/sendmail/sendmail: No such
>> file or directory
>> 
>> This patch removes this fallback code. I believe this is cleaner than
>> updating the fallback since we would have to put two paths in: one for
>> sendmail/send-mail/mailq and one for makemap/newaliases.
> 
> I am not sure about removing the fallback code but if we decide to
> keep it, this diff should fix the fallback case.

ok jung@ for the fix below.

I’m also unsure about the fallback code. It was introduced with -r1.4 more than 
16 yrs ago. However, I fail to understand what’s the point of it? I mean why 
running mailwrapper without a config file, that does not make much sense 
and defeats it's purpose, right? AFAIU, the mailer.conf is a must-have,
so the proposed removal of the fallback code makes sense to me.

> Index: mailwrapper.c
> ===
> RCS file: /cvs/src/usr.sbin/mailwrapper/mailwrapper.c,v
> retrieving revision 1.20
> diff -u -p -r1.20 mailwrapper.c
> --- mailwrapper.c 12 Oct 2015 22:01:08 -  1.20
> +++ mailwrapper.c 8 Dec 2015 09:36:18 -
> @@ -41,7 +41,7 @@
> #include 
> 
> #define _PATH_MAILERCONF  "/etc/mailer.conf"
> -#define _PATH_DEFAULTMTA "/usr/libexec/sendmail/sendmail"
> +#define _PATH_DEFAULTMTA "/usr/sbin/smtpctl"
> 
> struct arglist {
>   size_t argc, maxc;
> 



Re: patch 3/4 add generic keyboard backlight support

2015-12-08 Thread Joerg Jung
On Tue, Dec 08, 2015 at 10:48:47PM +0100, Mark Kettenis wrote:
> > Date: Mon, 7 Dec 2015 23:44:13 +0100
> > From: Joerg Jung <m...@umaxx.net>
> > 
> > Hi,
> > 
> > here comes the third part of the series for generic keyboard backlight
> > support.
> > 
> > Please find below a diff which adds they key(code)s for keyboard
> > backlight control, as found on all recent Intel based Apple Laptop
> > Keyboards.  While here, also add keys for display brightness control
> > found on Apple Keyboards as well.
> > 
> > This is based on a similar diff from Sven-Volker Nowarra and is a NOOP
> > for now, as these keys are not used yet. Using them is target for a
> > later diff.
> > 
> > I'm not familar with keycodes, most of the diff below is 'educated
> > guess', so please, hints are welcome!
> > 
> > Comments, OK?
> 
> Heh, I just looked into this during n2k15 to make the display
> brihtness keys work.

Did you got it working? :)

> I don't think you should add codes to pckbc/wskbdmap_mfii.c unless you
> have some evidence there really are PC-style keyboards that produce
> these codes.

Ok, understood. In my defence: as written above 'educated guess' meant,
I just copy here, using the same approach as the (already 
working) audio keys.
 
> I think you should choose different codes for USB.  Take a look at the
> "HID Usage Tables" document.  There you'll find that 131-134 are
> already assigned.  For my brightness diff I used codes in the reserved
> range that startx at 232 (0xe8).

Ok, makes sense.

> There are already keysyms defined for display brightness in "Group 4".

Yes, I have seen them.

> Using symbols in that group has some consequences though.  Currently
> the code that handles them assumes they are only generated in
> combination with certain modifies keys.

Yes, that is what I guessed and the reason why added them in the
previous group.

Modifier keys means  or , but does not include the famous
 key, right?

Now I'm a bit more confused: where and how should I add the backlight or
brightness keys to be able to access them in wskbd.c later?


> > Index: pckbc/wskbdmap_mfii.c
> > ===
> > RCS file: /cvs/src/sys/dev/pckbc/wskbdmap_mfii.c,v
> > retrieving revision 1.43
> > diff -u -p -r1.43 wskbdmap_mfii.c
> > --- pckbc/wskbdmap_mfii.c   14 Apr 2013 19:32:52 -  1.43
> > +++ pckbc/wskbdmap_mfii.c   7 Dec 2015 22:22:27 -
> > @@ -149,6 +149,10 @@ static const keysym_t pckbd_keydesc_us[]
> >  KC(170),   KS_Print_Screen,
> >  KC(174),   KS_AudioLower,
> >  KC(176),   KS_AudioRaise,
> > +KC(177),   KS_BrightnessDown,
> > +KC(178),   KS_BrightnessUp,
> > +KC(179),   KS_KbdBacklightDown,
> > +KC(180),   KS_KbdBacklightUp,
> >  KC(181),   KS_KP_Divide,
> >  KC(183),   KS_Print_Screen,
> >  KC(184), KS_Cmd2,  KS_Alt_R,   KS_Multi_key,
> > Index: wscons/wsksymdef.h
> > ===
> > RCS file: /cvs/src/sys/dev/wscons/wsksymdef.h,v
> > retrieving revision 1.36
> > diff -u -p -r1.36 wsksymdef.h
> > --- wscons/wsksymdef.h  26 Jan 2014 17:48:08 -  1.36
> > +++ wscons/wsksymdef.h  7 Dec 2015 22:22:27 -
> > @@ -633,6 +633,10 @@
> >  #define KS_AudioMute   0xf3d1
> >  #define KS_AudioLower  0xf3d2
> >  #define KS_AudioRaise  0xf3d3
> > +#define KS_BrightnessDown  0xf3d4
> > +#define KS_BrightnessUp0xf3d5
> > +#define KS_KbdBacklightDown0xf3d6
> > +#define KS_KbdBacklightUp  0xf3d7
> >  
> >  /*
> >   * Group 4 (command)
> > Index: usb/makemap.awk
> > ===
> > RCS file: /cvs/src/sys/dev/usb/makemap.awk,v
> > retrieving revision 1.14
> > diff -u -p -r1.14 makemap.awk
> > --- usb/makemap.awk 20 Nov 2013 17:27:32 -  1.14
> > +++ usb/makemap.awk 7 Dec 2015 22:22:27 -
> > @@ -153,6 +153,10 @@ BEGIN {
> > conv[170] = 70
> > conv[174] = 129
> > conv[176] = 128
> > +   conv[177] = 131
> > +   conv[178] = 132
> > +   conv[179] = 133
> > +   conv[180] = 134
> > conv[181] = 84
> > conv[184] = 230
> > # 198 is #if 0 in the PS/2 map...
> > Index: usb/ukbd.c
> > ==

Re: [PATCH] ukbd.c cleanup and mba iso support

2015-12-08 Thread Joerg Jung
On Tue, Dec 08, 2015 at 10:12:38PM +0100, Joerg Jung wrote:
> On Thu, Aug 06, 2015 at 09:58:56PM +0200, Joerg Jung wrote:
> > On Wed, Feb 18, 2015 at 10:33:57PM -0800, William Orr wrote:
> > > 
> > > Any interest?
> > 
> > I'm interested in this. Your diff looks reasonable, so I applied it and 
> > it compiled fine, but the less and grave keys are still wrong exchanged
> > on my MacBookAir4,2. Looks like usbdevs says I have a 
> > USB_PRODUCT_APPLE_WELLSPRING6_ISO:
> > 
> >port 2 addr 7: full speed, power 40 mA, config 1, Apple Internal
> >   Keyboard / Trackpad(0x024d), Apple Inc.(0x05ac), rev 2.09   
> > 
> > So I guess some more case's are required in the diff below?
> >  
> > > On 2/4/15 9:37 AM, William Orr wrote:
> > > > 
> > > > This implements some of Alexey's comments as well as munging the grave 
> > > > key for
> > > > macbook airs. Tested on a mba with a WELLSPRING ANSI keyboard.
> 
> Please find below an updated diff which also includes a case for the ISO
> keyboard found in my older MacbookAir4,2.
> 
> This newer diff also includes some minor KNF and whitespace cleanup.
> 
> I would really like to commit this, as this fixes a real bug (exchanged
> grave/less key). So no longer fiddling with xmodmap required.

Slightly updated diff below, sorting the switch cases in the same order
as found in usbdevs.
 
OK, anyone?


Index: sys/dev/usb/ukbd.c
===
RCS file: /cvs/src/sys/dev/usb/ukbd.c,v
retrieving revision 1.71
diff -u -p -r1.71 ukbd.c
--- sys/dev/usb/ukbd.c  14 Mar 2015 03:38:50 -  1.71
+++ sys/dev/usb/ukbd.c  8 Dec 2015 22:40:12 -
@@ -158,13 +158,13 @@ const struct wskbd_accessops ukbd_access
ukbd_ioctl,
 };
 
-int ukbd_match(struct device *, void *, void *); 
-void ukbd_attach(struct device *, struct device *, void *); 
-int ukbd_detach(struct device *, int); 
-
-struct cfdriver ukbd_cd = { 
-   NULL, "ukbd", DV_DULL 
-}; 
+intukbd_match(struct device *, void *, void *);
+void   ukbd_attach(struct device *, struct device *, void *);
+intukbd_detach(struct device *, int);
+
+struct cfdriver ukbd_cd = {
+   NULL, "ukbd", DV_DULL
+};
 
 const struct cfattach ukbd_ca = {
sizeof(struct ukbd_softc), ukbd_match, ukbd_attach, ukbd_detach
@@ -181,6 +181,9 @@ voidukbd_gdium_munge(void *, uint8_t *,
 void   ukbd_apple_munge(void *, uint8_t *, u_int);
 void   ukbd_apple_mba_munge(void *, uint8_t *, u_int);
 void   ukbd_apple_iso_munge(void *, uint8_t *, u_int);
+void   ukbd_apple_iso_mba_munge(void *, uint8_t *, u_int);
+void   ukbd_apple_translate(void *, uint8_t *, u_int,
+   const struct ukbd_translation *, u_int);
 uint8_tukbd_translate(const struct ukbd_translation *, size_t, 
uint8_t);
 
 int
@@ -241,17 +244,20 @@ ukbd_attach(struct device *parent, struc
switch (uha->uaa->product) {
case USB_PRODUCT_APPLE_FOUNTAIN_ISO:
case USB_PRODUCT_APPLE_GEYSER_ISO:
+   case USB_PRODUCT_APPLE_WELLSPRING6_ISO:
sc->sc_munge = ukbd_apple_iso_munge;
break;
-   case USB_PRODUCT_APPLE_WELLSPRING4A_ANSI:
-   case USB_PRODUCT_APPLE_WELLSPRING4A_ISO:
-   case USB_PRODUCT_APPLE_WELLSPRING4A_JIS:
-   case USB_PRODUCT_APPLE_WELLSPRING4_ANSI:
+   case USB_PRODUCT_APPLE_WELLSPRING_ISO:
case USB_PRODUCT_APPLE_WELLSPRING4_ISO:
-   case USB_PRODUCT_APPLE_WELLSPRING4_JIS:
+   case USB_PRODUCT_APPLE_WELLSPRING4A_ISO:
+   sc->sc_munge = ukbd_apple_iso_mba_munge;
+   break;
case USB_PRODUCT_APPLE_WELLSPRING_ANSI:
-   case USB_PRODUCT_APPLE_WELLSPRING_ISO:
case USB_PRODUCT_APPLE_WELLSPRING_JIS:
+   case USB_PRODUCT_APPLE_WELLSPRING4_ANSI:
+   case USB_PRODUCT_APPLE_WELLSPRING4_JIS:
+   case USB_PRODUCT_APPLE_WELLSPRING4A_ANSI:
+   case USB_PRODUCT_APPLE_WELLSPRING4A_JIS:
sc->sc_munge = ukbd_apple_mba_munge;
break;
default:
@@ -431,15 +437,14 @@ ukbd_cnpollc(void *v, int on)
 }
 
 void
-ukbd_cnbell(void *v, u_int pitch, u_int period, u_int volume) 
+ukbd_cnbell(void *v, u_int pi

Re: [PATCH] ukbd.c cleanup and mba iso support

2015-12-08 Thread Joerg Jung
Hi,

...back on this topic.

On Thu, Aug 06, 2015 at 09:58:56PM +0200, Joerg Jung wrote:
> On Wed, Feb 18, 2015 at 10:33:57PM -0800, William Orr wrote:
> > 
> > Any interest?
> 
> I'm interested in this. Your diff looks reasonable, so I applied it and 
> it compiled fine, but the less and grave keys are still wrong exchanged
> on my MacBookAir4,2. Looks like usbdevs says I have a 
> USB_PRODUCT_APPLE_WELLSPRING6_ISO:
> 
>port 2 addr 7: full speed, power 40 mA, config 1, Apple Internal
>   Keyboard / Trackpad(0x024d), Apple Inc.(0x05ac), rev 2.09   
> 
> So I guess some more case's are required in the diff below?
>  
> > On 2/4/15 9:37 AM, William Orr wrote:
> > > 
> > > This implements some of Alexey's comments as well as munging the grave 
> > > key for
> > > macbook airs. Tested on a mba with a WELLSPRING ANSI keyboard.

Please find below an updated diff which also includes a case for the ISO
keyboard found in my older MacbookAir4,2.

This newer diff also includes some minor KNF and whitespace cleanup.

I would really like to commit this, as this fixes a real bug (exchanged
grave/less key). So no longer fiddling with xmodmap required.

OK, anyone?

Thanks,
Regards,
Joerg


Index: sys/dev/usb/ukbd.c
===
RCS file: /cvs/src/sys/dev/usb/ukbd.c,v
retrieving revision 1.71
diff -u -p -r1.71 ukbd.c
--- sys/dev/usb/ukbd.c  14 Mar 2015 03:38:50 -  1.71
+++ sys/dev/usb/ukbd.c  8 Dec 2015 20:39:31 -
@@ -158,13 +158,13 @@ const struct wskbd_accessops ukbd_access
ukbd_ioctl,
 };
 
-int ukbd_match(struct device *, void *, void *); 
-void ukbd_attach(struct device *, struct device *, void *); 
-int ukbd_detach(struct device *, int); 
-
-struct cfdriver ukbd_cd = { 
-   NULL, "ukbd", DV_DULL 
-}; 
+intukbd_match(struct device *, void *, void *);
+void   ukbd_attach(struct device *, struct device *, void *);
+intukbd_detach(struct device *, int);
+
+struct cfdriver ukbd_cd = {
+   NULL, "ukbd", DV_DULL
+};
 
 const struct cfattach ukbd_ca = {
sizeof(struct ukbd_softc), ukbd_match, ukbd_attach, ukbd_detach
@@ -181,6 +181,9 @@ voidukbd_gdium_munge(void *, uint8_t *,
 void   ukbd_apple_munge(void *, uint8_t *, u_int);
 void   ukbd_apple_mba_munge(void *, uint8_t *, u_int);
 void   ukbd_apple_iso_munge(void *, uint8_t *, u_int);
+void   ukbd_apple_iso_mba_munge(void *, uint8_t *, u_int);
+void   ukbd_apple_translate(void *, uint8_t *, u_int,
+   const struct ukbd_translation *, u_int);
 uint8_tukbd_translate(const struct ukbd_translation *, size_t, 
uint8_t);
 
 int
@@ -243,14 +246,17 @@ ukbd_attach(struct device *parent, struc
case USB_PRODUCT_APPLE_GEYSER_ISO:
sc->sc_munge = ukbd_apple_iso_munge;
break;
-   case USB_PRODUCT_APPLE_WELLSPRING4A_ANSI:
+   case USB_PRODUCT_APPLE_WELLSPRING6_ISO:
case USB_PRODUCT_APPLE_WELLSPRING4A_ISO:
+   case USB_PRODUCT_APPLE_WELLSPRING4_ISO:
+   case USB_PRODUCT_APPLE_WELLSPRING_ISO:
+   sc->sc_munge = ukbd_apple_iso_mba_munge;
+   break;
+   case USB_PRODUCT_APPLE_WELLSPRING4A_ANSI:
case USB_PRODUCT_APPLE_WELLSPRING4A_JIS:
case USB_PRODUCT_APPLE_WELLSPRING4_ANSI:
-   case USB_PRODUCT_APPLE_WELLSPRING4_ISO:
case USB_PRODUCT_APPLE_WELLSPRING4_JIS:
case USB_PRODUCT_APPLE_WELLSPRING_ANSI:
-   case USB_PRODUCT_APPLE_WELLSPRING_ISO:
case USB_PRODUCT_APPLE_WELLSPRING_JIS:
sc->sc_munge = ukbd_apple_mba_munge;
break;
@@ -431,15 +437,14 @@ ukbd_cnpollc(void *v, int on)
 }
 
 void
-ukbd_cnbell(void *v, u_int pitch, u_int period, u_int volume) 
+ukbd_cnbell(void *v, u_int pitch, u_int period, u_int volume)
 {
hidkbd_bell(pitch, period, volume, 1);
-}  
+}
 
 int
 ukbd_cnattach(void)
 {
-
/*
 * XXX USB requires too many parts of the kernel to be running
 * XXX in order to work, so we can't do much for the console
@@ -460,12 +465,28 @@ ukbd_translate(const struct ukbd_transla
 }
 
 void
-ukbd_apple_munge(void *vsc, uint8_t *ibuf, u_int ilen)
+ukbd_apple_translate(void *vsc, uint8_t *ibuf, u_int ilen,
+const struct ukbd_translation* trans, u_int tlen)
 {
struct ukbd_softc *sc = vsc;
struct hidkbd *kbd = >sc_kbd;
uint8_t *pos, *spos, *epos, x

patch 2/4 add generic keyboard backlight support

2015-12-07 Thread Joerg Jung
Hi,

here comes the second part of the series for generic keyboard backlight
support.

Please find below the userland part which adds a backlight variable to
wsconsctl(8).

I have chosen to use the percentage format, so it looks like this:

   $ doas wsconsctl keyboard.backlight=80  
   keyboard.backlight -> 80.00%

Comments, OK?

Thanks,
Regards,
Joerg


Index: keyboard.c
===
RCS file: /cvs/src/sbin/wsconsctl/keyboard.c,v
retrieving revision 1.12
diff -u -p -r1.12 keyboard.c
--- keyboard.c  21 Mar 2013 06:04:05 -  1.12
+++ keyboard.c  7 Dec 2015 21:03:22 -
@@ -50,6 +50,7 @@ static struct wskbd_keyrepeat_data repea
 static struct wskbd_keyrepeat_data dfrepeat;
 static int ledstate;
 static kbd_t kbdencoding;
+static struct field_pc backlight;
 
 struct field keyboard_field_tab[] = {
 { "type",  ,FMT_KBDTYPE,FLG_RDONLY },
@@ -66,12 +67,15 @@ struct field keyboard_field_tab[] = {
 { "repeat.deln.default",   , FMT_UINT,   FLG_MODIFY },
 { "ledstate",  ,  FMT_UINT,   0 },
 { "encoding",  ,   FMT_KBDENC, FLG_MODIFY },
+{ "backlight", , FMT_PC, 
FLG_MODIFY|FLG_INIT },
 { NULL }
 };
 
 void
 keyboard_get_values(int fd)
 {
+   struct wskbd_backlight kbl;
+
if (field_by_value(keyboard_field_tab, )->flags & FLG_GET)
if (ioctl(fd, WSKBDIO_GTYPE, ) < 0)
warn("WSKBDIO_GTYPE");
@@ -131,11 +135,24 @@ keyboard_get_values(int fd)
if (field_by_value(keyboard_field_tab, )->flags & FLG_GET)
if (ioctl(fd, WSKBDIO_GETENCODING, ) < 0)
warn("WSKBDIO_GETENCODING");
+
+   if (field_by_value(keyboard_field_tab, )->flags & FLG_GET) {
+   if (ioctl(fd, WSKBDIO_GETBACKLIGHT, ) < 0)
+   warn("WSKBDIO_GETBACKLIGHT");
+   else {
+backlight.min = kbl.min;
+backlight.cur = kbl.curval;
+backlight.max = kbl.max;
+   }
+   }
+   
 }
 
 int
 keyboard_put_values(int fd)
 {
+   struct wskbd_backlight kbl;
+
bell.which = 0;
if (field_by_value(keyboard_field_tab, )->flags & FLG_SET)
bell.which |= WSKBD_BELL_DOPITCH;
@@ -200,6 +217,16 @@ keyboard_put_values(int fd)
if (field_by_value(keyboard_field_tab, )->flags & FLG_SET) {
if (ioctl(fd, WSKBDIO_SETENCODING, ) < 0) {
warn("WSKBDIO_SETENCODING");
+   return 1;
+   }
+   }
+
+   if (field_by_value(keyboard_field_tab, )->flags & FLG_SET) {
+   kbl.min = backlight.min;
+   kbl.curval = backlight.cur;
+   kbl.max = backlight.max;
+   if (ioctl(fd, WSKBDIO_SETBACKLIGHT, ) < 0) {
+   warn("WSKBDIO_SETBACKLIGHT");
return 1;
}
}



patch 1/4 add generic keyboard backlight support

2015-12-07 Thread Joerg Jung
Hi,

here comes a series of small diffs which add generic support for
keyboard backlights.

Please find below the first diff, which adds new ioctls to wskbd(4) to
control keyboard backlights.

In contrast to an earlier diff from jcs, I have chosen to use a struct
in favor of a simple (unsigned) int as depending on the vendor
(Thinkpad, Apple, ...) different min/max values for the brightness of
the keyboard backlight are possible.

Comments, OK?

Thanks,
Regards,
Joerg



Index: wsconsio.h
===
RCS file: /cvs/src/sys/dev/wscons/wsconsio.h,v
retrieving revision 1.72
diff -u -p -r1.72 wsconsio.h
--- wsconsio.h  30 Aug 2015 10:05:09 -  1.72
+++ wsconsio.h  7 Dec 2015 21:03:45 -
@@ -180,6 +180,13 @@ struct wskbd_map_data {
 #define WSKBDIO_GETENCODING_IOR('W', 15, kbd_t)
 #define WSKBDIO_SETENCODING_IOW('W', 16, kbd_t)
 
+/* Get/set keyboard backlight.  Not applicable to all keyboard types. */
+struct wskbd_backlight {
+   unsigned int min, max, curval;
+};
+#defineWSKBDIO_GETBACKLIGHT_IOR('W', 17, struct wskbd_backlight)
+#defineWSKBDIO_SETBACKLIGHT_IOW('W', 18, struct wskbd_backlight)
+
 /* internal use only */
 #define WSKBDIO_SETMODE_IOW('W', 19, int)
 #define WSKBDIO_GETMODE_IOR('W', 20, int)
Index: wskbd.c
===
RCS file: /cvs/src/sys/dev/wscons/wskbd.c,v
retrieving revision 1.82
diff -u -p -r1.82 wskbd.c
--- wskbd.c 10 Sep 2015 18:14:52 -  1.82
+++ wskbd.c 7 Dec 2015 21:03:45 -
@@ -230,6 +230,9 @@ int wskbd_mux_close(struct wsevsrc *);
 intwskbd_do_open(struct wskbd_softc *, struct wseventvar *);
 intwskbd_do_ioctl(struct device *, u_long, caddr_t, int, struct proc *);
 
+int(*wskbd_get_backlight)(struct wskbd_backlight *);
+int(*wskbd_set_backlight)(struct wskbd_backlight *);
+
 struct cfdriver wskbd_cd = {
NULL, "wskbd", DV_TTY
 };
@@ -1010,6 +1013,7 @@ wskbd_displayioctl(struct device *dev, u
case WSKBDIO_SETDEFAULTKEYREPEAT:
case WSKBDIO_SETMAP:
case WSKBDIO_SETENCODING:
+   case WSKBDIO_SETBACKLIGHT:
if ((flag & FWRITE) == 0)
return (EACCES);
}
@@ -1155,6 +1159,18 @@ getkeyrepeat:
wsmux_set_layout(sc->sc_base.me_parent, enc);
 #endif
return (0);
+
+   case WSKBDIO_GETBACKLIGHT:
+   if (wskbd_get_backlight != NULL)
+   return (*wskbd_get_backlight)((struct wskbd_backlight 
*)data);
+   error = ENOTTY;
+   break;
+
+   case WSKBDIO_SETBACKLIGHT:
+   if (wskbd_set_backlight != NULL)
+   return (*wskbd_set_backlight)((struct wskbd_backlight 
*)data);
+   error = ENOTTY;
+   break;
}
 
/*



patch 3/4 add generic keyboard backlight support

2015-12-07 Thread Joerg Jung
Hi,

here comes the third part of the series for generic keyboard backlight
support.

Please find below a diff which adds they key(code)s for keyboard
backlight control, as found on all recent Intel based Apple Laptop
Keyboards.  While here, also add keys for display brightness control
found on Apple Keyboards as well.

This is based on a similar diff from Sven-Volker Nowarra and is a NOOP
for now, as these keys are not used yet. Using them is target for a
later diff.

I'm not familar with keycodes, most of the diff below is 'educated
guess', so please, hints are welcome!

Comments, OK?

Thanks,
Regards,
Joerg


Index: pckbc/wskbdmap_mfii.c
===
RCS file: /cvs/src/sys/dev/pckbc/wskbdmap_mfii.c,v
retrieving revision 1.43
diff -u -p -r1.43 wskbdmap_mfii.c
--- pckbc/wskbdmap_mfii.c   14 Apr 2013 19:32:52 -  1.43
+++ pckbc/wskbdmap_mfii.c   7 Dec 2015 22:22:27 -
@@ -149,6 +149,10 @@ static const keysym_t pckbd_keydesc_us[]
 KC(170),   KS_Print_Screen,
 KC(174),   KS_AudioLower,
 KC(176),   KS_AudioRaise,
+KC(177),   KS_BrightnessDown,
+KC(178),   KS_BrightnessUp,
+KC(179),   KS_KbdBacklightDown,
+KC(180),   KS_KbdBacklightUp,
 KC(181),   KS_KP_Divide,
 KC(183),   KS_Print_Screen,
 KC(184), KS_Cmd2,  KS_Alt_R,   KS_Multi_key,
Index: wscons/wsksymdef.h
===
RCS file: /cvs/src/sys/dev/wscons/wsksymdef.h,v
retrieving revision 1.36
diff -u -p -r1.36 wsksymdef.h
--- wscons/wsksymdef.h  26 Jan 2014 17:48:08 -  1.36
+++ wscons/wsksymdef.h  7 Dec 2015 22:22:27 -
@@ -633,6 +633,10 @@
 #define KS_AudioMute   0xf3d1
 #define KS_AudioLower  0xf3d2
 #define KS_AudioRaise  0xf3d3
+#define KS_BrightnessDown  0xf3d4
+#define KS_BrightnessUp0xf3d5
+#define KS_KbdBacklightDown0xf3d6
+#define KS_KbdBacklightUp  0xf3d7
 
 /*
  * Group 4 (command)
Index: usb/makemap.awk
===
RCS file: /cvs/src/sys/dev/usb/makemap.awk,v
retrieving revision 1.14
diff -u -p -r1.14 makemap.awk
--- usb/makemap.awk 20 Nov 2013 17:27:32 -  1.14
+++ usb/makemap.awk 7 Dec 2015 22:22:27 -
@@ -153,6 +153,10 @@ BEGIN {
conv[170] = 70
conv[174] = 129
conv[176] = 128
+   conv[177] = 131
+   conv[178] = 132
+   conv[179] = 133
+   conv[180] = 134
conv[181] = 84
conv[184] = 230
# 198 is #if 0 in the PS/2 map...
Index: usb/ukbd.c
===
RCS file: /cvs/src/sys/dev/usb/ukbd.c,v
retrieving revision 1.71
diff -u -p -r1.71 ukbd.c
--- usb/ukbd.c  14 Mar 2015 03:38:50 -  1.71
+++ usb/ukbd.c  7 Dec 2015 22:22:27 -
@@ -469,13 +469,15 @@ ukbd_apple_munge(void *vsc, uint8_t *ibu
static const struct ukbd_translation apple_fn_trans[] = {
{ 40, 73 }, /* return -> insert */
{ 42, 76 }, /* backspace -> delete */
+   { 58, 131 },/* F1 -> screen brightness down */
+   { 59, 132 },/* F2 -> screen brightness up */
 #ifdef notyet
-   { 58, 0 },  /* F1 -> screen brightness down */
-   { 59, 0 },  /* F2 -> screen brightness up */
{ 60, 0 },  /* F3 */
{ 61, 0 },  /* F4 */
-   { 62, 0 },  /* F5 -> keyboard backlight down */
-   { 63, 0 },  /* F6 -> keyboard backlight up */
+#endif
+   { 62, 133 },/* F5 -> keyboard backlight down */
+   { 63, 134 },/* F6 -> keyboard backlight up */
+#ifdef notyet
{ 64, 0 },  /* F7 -> audio back */
{ 65, 0 },  /* F8 -> audio pause/play */
{ 66, 0 },  /* F9 -> audio next */
Index: usb/ukbdmap.c
===
RCS file: /cvs/src/sys/dev/usb/ukbdmap.c,v
retrieving revision 1.41
diff -u -p -r1.41 ukbdmap.c
--- usb/ukbdmap.c   20 Nov 2013 17:28:00 -  1.41
+++ usb/ukbdmap.c   7 Dec 2015 22:22:27 -
@@ -1,4 +1,4 @@
-/* $OpenBSD: ukbdmap.c,v 1.41 2013/11/20 17:28:00 miod Exp $   */
+/* $OpenBSD$   */
 
 /*
  * THIS FILE IS AUTOMAGICALLY GENERATED.  DO NOT EDIT.
@@ -176,6 +176,10 @@ static const keysym_t ukbd_keydesc_us[] 
 KC(127),   KS_AudioMute,
 KC(128),   KS_AudioRaise,
 KC(129),   KS_AudioLower,
+KC(131),   KS_BrightnessDown,
+KC(132),   KS_BrightnessUp,
+KC(133),   KS_KbdBacklightDown,
+KC(134),   KS_KbdBacklightUp,
 KC(224),   KS_Cmd1,KS_Control_L,
 KC(225),   KS_Shift_L,
 KC(226),   KS_Cmd2,KS_Alt_L,



patch 4/4 add keyboard backlight support to asmc(4)

2015-12-07 Thread Joerg Jung
Hi,

here comes another diff of the series for the keyboard backlight
support.

Please find below a diff which enables keyboard backlight control on
Intel Apple Laptops via asmc(4).

This diff uses introduced wskbd(4) hooks from previously sent diffs and
also introduces locking and minor refactoring around asmc_try().

Comments, tests, OKs?

Thanks,
Regards,
Joerg


Index: asmc.c
===
RCS file: /cvs/src/sys/dev/isa/asmc.c,v
retrieving revision 1.13
diff -u -p -r1.13 asmc.c
--- asmc.c  29 Oct 2015 13:29:04 -  1.13
+++ asmc.c  7 Dec 2015 22:50:02 -
@@ -23,12 +23,14 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
 #include 
 
 #include 
+#include 
 
 #define ASMC_BASE  0x300   /* SMC base address */
 #define ASMC_IOSIZE32  /* I/O region size 0x300-0x31f */
@@ -66,10 +68,13 @@ struct asmc_softc {
uint8_t  sc_init;   /* initialization done? */
uint8_t  sc_nfans;  /* number of fans */
uint8_t  sc_lightlen;   /* light data len */
+   uint8_t  sc_kbdled; /* backlight led value */
 
+   struct rwlocksc_lock;
struct taskq*sc_taskq;
struct task  sc_task_init;
struct task  sc_task_refresh;
+   struct task  sc_task_backlight;
 
struct ksensor   sc_sensor_temp[ASMC_MAXTEMP];
struct ksensor   sc_sensor_fan[ASMC_MAXFAN];
@@ -81,7 +86,6 @@ struct asmc_softc {
 
 uint8_tasmc_status(struct asmc_softc *);
 intasmc_try(struct asmc_softc *, int, const char *, uint8_t *, uint8_t);
-void   asmc_kbdled(struct asmc_softc *, uint8_t);
 
 void   asmc_init(void *);
 void   asmc_refresh(void *);
@@ -91,6 +95,13 @@ int  asmc_match(struct device *, void *, 
 void   asmc_attach(struct device *, struct device *, void *);
 intasmc_detach(struct device *, int);
 
+/* wskbd hook functions */
+void   asmc_kbdled(void *);
+intasmc_get_backlight(struct wskbd_backlight *);
+intasmc_set_backlight(struct wskbd_backlight *);
+extern int (*wskbd_get_backlight)(struct wskbd_backlight *);
+extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
+
 const struct cfattach asmc_ca = {
sizeof(struct asmc_softc), asmc_match, asmc_attach
 };
@@ -256,7 +267,7 @@ asmc_attach(struct device *parent, struc
struct asmc_softc *sc = (struct asmc_softc *)self;
struct isa_attach_args *ia = aux;
uint8_t buf[6];
-   int i;
+   int i, r;
 
if (bus_space_map(ia->ia_iot, ia->ia_iobase, ia->ia_iosize, 0,
>sc_ioh)) {
@@ -265,23 +276,34 @@ asmc_attach(struct device *parent, struc
}
sc->sc_iot = ia->ia_iot;
 
-   if (asmc_try(sc, ASMC_READ, "REV ", buf, 6)) {
-   printf(": revision failed (0x%x)\n", asmc_status(sc));
+   rw_init(>sc_lock, sc->sc_dev.dv_xname);
+
+   if ((r = asmc_try(sc, ASMC_READ, "REV ", buf, 6))) {
+   printf(": revision failed (0x%x)\n", r);
bus_space_unmap(ia->ia_iot, ia->ia_iobase, ASMC_IOSIZE);
return;
}
printf(": rev %x.%x%x%x", buf[0], buf[1], buf[2],
ntohs(*(uint16_t *)buf + 4));
 
-   if (asmc_try(sc, ASMC_READ, "#KEY", buf, 4)) {
-   printf(", no of keys failed (0x%x)\n", asmc_status(sc));
+   if ((r = asmc_try(sc, ASMC_READ, "#KEY", buf, 4))) {
+   printf(", no of keys failed (0x%x)\n", r);
bus_space_unmap(ia->ia_iot, ia->ia_iobase, ASMC_IOSIZE);
return;
}
printf(", %u key%s\n", ntohl(*(uint32_t *)buf),
(ntohl(*(uint32_t *)buf) == 1) ? "" : "s");
 
-   asmc_kbdled(sc, 127);
+   /* keyboard backlight led is optional */
+   sc->sc_kbdled = buf[0] = 127, buf[1] = 0;
+   if ((r = asmc_try(sc, ASMC_WRITE, "LKSB", buf, 2))) {
+   if (r != ASMC_NOTFOUND)
+   printf("%s: keyboard backlight failed (0x%x)\n",
+   sc->sc_dev.dv_xname, r);
+   } else {
+   wskbd_get_backlight = asmc_get_backlight;
+   wskbd_set_backlight = asmc_set_backlight;
+   }
 
if (!(sc->sc_taskq = taskq_create("asmc", 1, IPL_NONE, 0))) {
printf("%s: can't create task queue\n", sc->sc_dev.dv_xname);
@@ -290,6 +312,7 @@ asmc_attach(struct device *parent, struc
}
task_set(>sc_task_init, asmc_init, sc);
task_set(>sc_task_refresh, asmc_refresh, sc);
+   task_set(>sc_task_backlight, asmc_kbdled, sc);
 
strlcpy(sc->sc_sensor_dev.xname, sc->sc_dev.dv_xname,
sizeof(sc->sc_sensor_dev.xname));
@@ -327,6 +350,7 @@ int
 asmc_detach(struct device *self, int flags)
 {
struct asmc_softc *sc = (struct asmc_softc *)self;
+   uint8_t buf[2] = { (sc->sc_kbdled = 0), 0 };
int i;
 
   

Re: calloc -> malloc in get_data() and get_string()

2015-10-28 Thread Joerg Jung


> Am 28.10.2015 um 17:05 schrieb Michael McConville :
> 
> Relayd, httpd, and ntpd define the functions get_data() and
> get_string(). Both call calloc and then immediately memcpy. Calloc's
> zeroing isn't optimized out. These functions are called in network data
> paths in at least a couple places, so all this extra writing could have
> a meaningful performance impact.

I think this impact is negligible small and I believe
that you can not even measure it.
So this change makes not so much sense to me.

I wonder if using strndup() would make more 
sense for the first case here?

> ok?
> 
> 
> Index: relayd.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
> retrieving revision 1.144
> diff -u -p -r1.144 relayd.c
> --- relayd.c14 Oct 2015 07:58:14 -1.144
> +++ relayd.c28 Oct 2015 15:57:16 -
> @@ -1501,9 +1501,10 @@ get_string(u_int8_t *ptr, size_t len)
>isspace((unsigned char)ptr[i])))
>break;
> 
> -if ((str = calloc(1, i + 1)) == NULL)
> +if ((str = malloc(i + 1)) == NULL)
>return (NULL);
>memcpy(str, ptr, i);
> +str[i] = '\0';
> 
>return (str);
> }
> @@ -1513,7 +1514,7 @@ get_data(u_int8_t *ptr, size_t len)
> {
>u_int8_t*data;
> 
> -if ((data = calloc(1, len)) == NULL)
> +if ((data = malloc(len)) == NULL)
>return (NULL);
>memcpy(data, ptr, len);
> 
> 



Re: WAPBL implementation

2015-10-23 Thread Joerg Jung
On Fri, Oct 23, 2015 at 01:19:15PM -0200, Walter Neto wrote:
> Like recommended from other developers I started developing WAPBL support for
> OpenBSD.
> 
> Looking at NetBSD and Bitrig I mage a first funcional patch.

Wow... that is a big diff :)
Care to elaborate in some more words what "functional" means here?

Thanks,
Regards,
Joerg

> Index: sbin/mount/mntopts.h
> ===
> RCS file: /Volumes/CSP/cvs/src/sbin/mount/mntopts.h,v
> retrieving revision 1.16
> diff -u -r1.16 mntopts.h
> --- sbin/mount/mntopts.h  13 Jul 2014 12:01:30 -  1.16
> +++ sbin/mount/mntopts.h  23 Oct 2015 15:07:07 -
> @@ -66,6 +66,8 @@
>   | MFLAG_OPT }
>  #define MOPT_SOFTDEP { "softdep",MNT_SOFTDEP, MFLAG_SET }
>  
> +#define MOPT_LOG { "log",MNT_LOG, MFLAG_SET }
> +
>  /* Control flags. */
>  #define MOPT_FORCE   { "force",  MNT_FORCE, MFLAG_SET }
>  #define MOPT_UPDATE  { "update", MNT_UPDATE, MFLAG_SET }
> Index: sbin/mount/mount.c
> ===
> RCS file: /Volumes/CSP/cvs/src/sbin/mount/mount.c,v
> retrieving revision 1.60
> diff -u -r1.60 mount.c
> --- sbin/mount/mount.c16 Jan 2015 06:39:59 -  1.60
> +++ sbin/mount/mount.c23 Oct 2015 15:07:07 -
> @@ -94,6 +94,7 @@
>   { MNT_ROOTFS,   1,  "root file system", "" },
>   { MNT_SYNCHRONOUS,  0,  "synchronous",  "sync" },
>   { MNT_SOFTDEP,  0,  "softdep",  "softdep" },
> + { MNT_LOG,  0,  "log",  "log" },
>   { 0,0,  "", "" }
>  };
>  
> Index: sbin/mount_ffs/mount_ffs.c
> ===
> RCS file: /Volumes/CSP/cvs/src/sbin/mount_ffs/mount_ffs.c,v
> retrieving revision 1.21
> diff -u -r1.21 mount_ffs.c
> --- sbin/mount_ffs/mount_ffs.c16 Jan 2015 06:39:59 -  1.21
> +++ sbin/mount_ffs/mount_ffs.c23 Oct 2015 15:07:07 -
> @@ -53,6 +53,7 @@
>   MOPT_RELOAD,
>   MOPT_FORCE,
>   MOPT_SOFTDEP,
> + MOPT_LOG,
>   { NULL }
>  };
>  
> Index: sys/conf/GENERIC
> ===
> RCS file: /Volumes/CSP/cvs/src/sys/conf/GENERIC,v
> retrieving revision 1.220
> diff -u -r1.220 GENERIC
> --- sys/conf/GENERIC  10 Aug 2015 20:35:36 -  1.220
> +++ sys/conf/GENERIC  23 Oct 2015 15:07:07 -
> @@ -43,6 +43,7 @@
>  option   FIFO# FIFOs; RECOMMENDED
>  option   TMPFS   # efficient memory file system
>  option   FUSE# FUSE
> +option   WAPBL   # Write Ahead Physical Block Logging
>  
>  option   SOCKET_SPLICE   # Socket Splicing for TCP and UDP
>  option   TCP_SACK# Selective Acknowledgements for TCP
> Index: sys/conf/files
> ===
> RCS file: /Volumes/CSP/cvs/src/sys/conf/files,v
> retrieving revision 1.604
> diff -u -r1.604 files
> --- sys/conf/files9 Oct 2015 01:17:21 -   1.604
> +++ sys/conf/files23 Oct 2015 15:07:07 -
> @@ -732,6 +732,7 @@
>  file kern/vfs_vops.c
>  file kern/vfs_vnops.c
>  file kern/vfs_getcwd.c
> +file kern/vfs_wapbl.cwapbl
>  file kern/spec_vnops.c
>  file miscfs/deadfs/dead_vnops.c
>  file miscfs/fifofs/fifo_vnops.c  fifo
> @@ -887,6 +888,7 @@
>  file ufs/ffs/ffs_vfsops.cffs | mfs
>  file ufs/ffs/ffs_vnops.c ffs | mfs
>  file ufs/ffs/ffs_softdep.c   ffs_softupdates
> +file ufs/ffs/ffs_wapbl.c ffs & wapbl
>  file ufs/mfs/mfs_vfsops.cmfs
>  file ufs/mfs/mfs_vnops.c mfs
>  file ufs/ufs/ufs_bmap.c  ffs | mfs | ext2fs
> @@ -898,6 +900,7 @@
>  file ufs/ufs/ufs_quota_stub.cffs | mfs
>  file ufs/ufs/ufs_vfsops.cffs | mfs | ext2fs
>  file ufs/ufs/ufs_vnops.c ffs | mfs | ext2fs
> +file ufs/ufs/ufs_wapbl.c ffs & wapbl
>  file ufs/ext2fs/ext2fs_alloc.c   ext2fs
>  file ufs/ext2fs/ext2fs_balloc.c  ext2fs
>  file ufs/ext2fs/ext2fs_bmap.cext2fs
> Index: sys/kern/spec_vnops.c
> ===
> RCS file: /Volumes/CSP/cvs/src/sys/kern/spec_vnops.c,v
> retrieving revision 1.83
> diff -u -r1.83 spec_vnops.c
> --- sys/kern/spec_vnops.c 10 Feb 2015 21:56:09 -  1.83
> +++ sys/kern/spec_vnops.c 23 Oct 2015 15:07:07 -
> @@ -408,6 +408,10 @@
>   return (EOPNOTSUPP);
>  }
>  
> +#ifdef WAPBL
> +extern int ffs_wapbl_fsync_vfs(struct vnode *, int);
> +#endif
> +
>  /*
>   * Synch buffers associated with a block device
>   */
> @@ -422,6 +426,15 @@
>  
>   if (vp->v_type == VCHR)
>   

Re: CVS: cvs.openbsd.org: src

2015-10-23 Thread Joerg Jung
On Wed, Sep 30, 2015 at 06:07:54PM +0200, Joerg Jung wrote:
> On Wed, Sep 30, 2015 at 05:13:31PM +0200, Martijn van Duren wrote:
> > On 09/30/15 14:15, Joerg Jung wrote:
> 
> > Thanks! Although it does add about 5-10 seconds to my boot-time, waiting
> > primarily for the temperature sensors.
> 
> Yes, they are probed during startup. That probing is slow, as the SMC is
> slow.  This needs some rethinking/polishing, moving the probing to a
> later point.

tl;dr: please find below a refactoring diff that fixes this.


The -current asmc(4) code suffers from two problems:

I.  The initial sensors read may take several seconds.
II. The update of the sensor values tsleep()s excessively within the
sensor task.

The following suggestions and hints to fix these timing issues were
provided to me:

1. mpi@ and Theo recommended not to tsleep() within the sensor task,
   mpi@ says: "Sleeping in the sensor thread might add latency to others
  sensors discovery."

   -> This is totally right, and matches II. problem statement above.

2. Theo suggested an async model using the sensor task (similar to
   ugold(4)), something like: sensor task comes, some command is issued,
   task returns, comes back later, reads result, next sensor, ...

   -> This sounds promising in theory, but I played a bit with this
  approach and it seems to be impossible to implement with asmc(4).
   -> The problem is that SMC requires several read() and write()
  operations for one command/sensor read, all of them acknowledged
  with waiting for the correct status, the protocol looks like this:
 1. write command, wait for status "accepted",
 2. write key, 4 single byte writes, each wait for status "accepted"
 3. write data length expected to read, wait for status "accepted"
 4. finally read data values, single byte reads each waiting for
status "ready for read"
  Step 1-3 might take in the seldom worst case several seconds 
  (retries + timeout).  I can not see an easy way to do this async
  and step-by-step as waiting too long will result in
  "comm collision" errors or might take ages, as it reads values
  from up to 100 sensors using these steps.

So, I came to the conclusion that solving I.+II. really requires an
additional thread.

3. uebayasi@ suggested to look into sys/dev/ipmi.c, which uses a kthread
   to initially read all sensor values from a slow serial bus.

   -> This was a good hint, related ipmi code is simple and it can be
  easily done in the same way in asmc(4), but according to a quick
  grep it is the ONLY sensor driver which does it in this way, and
  moreover:

4. tedu@ says: "oh, don't make your own thread unless you really need
   the loop. you can create your own taskq as well"

   -> Makes sense, a quick grep revealed, that (again) ONLY one single
  driver does it like this: viomb(4), again using an easy pattern to
  copy into asmc(4).

So, finally the refactoring diff below follows tedu's suggestion and
uses an own taskq to solve I.+II.

Tests, comments and OKs are welcome.

Thanks,
Regards,
Joerg



Index: asmc.c
===
RCS file: /cvs/src/sys/dev/isa/asmc.c,v
retrieving revision 1.11
diff -u -p -r1.11 asmc.c
--- asmc.c  15 Oct 2015 01:14:33 -  1.11
+++ asmc.c  23 Oct 2015 13:24:34 -
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -62,7 +63,13 @@ struct asmc_softc {
bus_space_handle_t   sc_ioh;
 
struct asmc_prod*sc_prod;
-   uint8_t  sc_lightlen;   /* light data len */
+   uint8_t  sc_init;   /* initialization done? */
+   uint8_t  sc_nfans;  /* number of fans */
+   uint8_t  sc_lightlen;   /* light data len */
+
+   struct taskq*sc_taskq;
+   struct task  sc_task_init;
+   struct task  sc_task_refresh;
 
struct ksensor   sc_sensor_temp[ASMC_MAXTEMP];
struct ksensor   sc_sensor_fan[ASMC_MAXFAN];
@@ -72,7 +79,12 @@ struct asmc_softc {
struct sensor_task  *sc_sensor_task;
 };
 
-intasmc_init(struct asmc_softc *);
+uint8_tasmc_status(struct asmc_softc *);
+intasmc_try(struct asmc_softc *, int, const char *, uint8_t *, uint8_t);
+void   asmc_kbdled(struct asmc_softc *, uint8_t);
+
+void   asmc_init(void *);
+void   asmc_refresh(void *);
 void   asmc_update(void *);
 
 intasmc_match(struct device *, void *, void *);
@@ -243,6 +255,8 @@ asmc_attach(struct device *parent, struc
 {
struct asmc_softc *sc = (struct asmc_softc *)self;
struct isa_attach_args *ia = aux;
+   uint8_t buf[6];
+   int i;
 
if (bus_space_map(ia-

Re: ftp: ctype interfaces need unsigned chars

2015-10-11 Thread Joerg Jung

> Am 11.10.2015 um 04:38 schrieb Philip Guenther :
> 
>> --- smtpd/rfc2822.c
>> +++ /tmp/cocci-output-29655-69b554-rfc2822.c
>> @@ -93,13 +93,13 @@ parser_feed_header(struct rfc2822_parser
>>char*pos;
>> 
>>/* new header */
>> -if (! isspace(*line) && *line != '\0') {
>> +if (! isspace((unsigned char)*line) && *line != '\0') {
> 
> Yep

I would remove the space between '!' and 'isspace()'.

>> -if (isspace(*(pos + 1)))
>> +if (isspace((unsigned char)*(pos + 1)))
> 
> Again, array indexing reads better here, IMO:
>if (isspace((unsigned char)pos[1])

Yes.

>> @@ -169,7 +169,7 @@ rfc2822_parser_feed(struct rfc2822_parse
>>charbuffer[RFC2822_MAX_LINE_SIZE+1];
>> 
>>/* in header and line is not a continuation, execute callback */
>> -if (rp->in_hdr && (*line == '\0' || !isspace(*line)))
>> +if (rp->in_hdr && (*line == '\0' || !isspace((unsigned char)*line)))
> 
> Yep.
> 
> ok?

ok jung@



Re: CVS: cvs.openbsd.org: src

2015-10-04 Thread Joerg Jung
On Wed, Sep 30, 2015 at 06:07:54PM +0200, Joerg Jung wrote:
> On Wed, Sep 30, 2015 at 05:13:31PM +0200, Martijn van Duren wrote:
> > 
> > What I find somewhat strange is that although the dmesg says it has "2
> > lights", it only shows one illuminance sensor in my sysctl. 
> 
> This is expected. In newer models the SMC shows up with two sensor keys,
> but only *one* provides useful/valid values (with 10bit instead of 6bit
> in older models). I probably should remove the number from the initial
> message to avoid confusion.

I committed a fix to -current, so that it should show now the correct
number of light sensors on initialization.



Re: CVS: cvs.openbsd.org: src

2015-10-04 Thread Joerg Jung
On Sun, Oct 04, 2015 at 09:20:06PM +0200, Mark Kettenis wrote:
> 
> Here is what I get on the Macmini 1,1 now:
> 
> asmc0 at isa0 port 0x300/32
> asmc0: rev 1.3f503, 137 keys, 5 temperatures, 1 fan, 0 lights, kbdled

Thanks for testing! I think printing "0 lights" is not very useful,
so I will tweak the output a bit further.
 
> hw.sensors.cpu0.temp0=54.00 degC
> hw.sensors.asmc0.temp0=56.00 degC (TC0D CPU0 Die Core)
> hw.sensors.asmc0.temp1=51.00 degC (TC0H CPU0 Heatsink)
> hw.sensors.asmc0.temp2=55.00 degC (TC0P CPU0 Proximity)
> hw.sensors.asmc0.temp3=54.00 degC (TN0P Northbridge Proximity)
> hw.sensors.asmc0.temp4=55.00 degC (TN1P Northbridge 2)
> hw.sensors.asmc0.fan0=1538 RPM (Master)
> 
> To answer your earlier question: I think having asmc(4) provide the
> Die Core temperature is good even though it is redundant.  This allows
> us to verify that cpu(4) is providing the right temperature/

Ok, fair enough.



Re: CVS: cvs.openbsd.org: src

2015-09-30 Thread Joerg Jung
On Wed, Sep 30, 2015 at 05:13:31PM +0200, Martijn van Duren wrote:
> On 09/30/15 14:15, Joerg Jung wrote:
> >CVSROOT: /cvs
> >Module name: src
> >Changes by:  j...@cvs.openbsd.org2015/09/30 06:15:12
> >
> >Modified files:
> > share/man/man4 : isa.4
> > sys/arch/amd64/conf: GENERIC
> > sys/arch/i386/conf: GENERIC
> > sys/dev/isa: files.isa
> >Added files:
> > share/man/man4 : asmc.4
> > sys/dev/isa: asmc.c
> >
> >Log message:
> >add a (disabled) driver for the Apple System Management Controller (SMC) as
> >found in Apple Intel based devices
> >
> >"go at it" deraadt@
> >
> Just backported this driver to my 5.7-stable system at work (yes, I like to
> live dangerous from time to time) and the driver seems to work properly.

:)

> Thanks! Although it does add about 5-10 seconds to my boot-time, waiting
> primarily for the temperature sensors.

Yes, they are probed during startup. That probing is slow, as the SMC is
slow.  This needs some rethinking/polishing, moving the probing to a
later point.

> See sysctl and dmesg output below.
> 
> What I find somewhat strange is that although the dmesg says it has "2
> lights", it only shows one illuminance sensor in my sysctl. 

This is expected. In newer models the SMC shows up with two sensor keys,
but only *one* provides useful/valid values (with 10bit instead of 6bit
in older models). I probably should remove the number from the initial
message to avoid confusion.

> This does
> somewhat seems to fit with the right side of my screen being marginally
> darker then the left side of my screen. Although this is something that
> doesn't bother me in the least it seem like something that might be worth
> mentioning.
> 
> $ sysctl | grep asmc
> hw.sensors.asmc0.temp0=30.00 degC (TA0P Ambient)
> hw.sensors.asmc0.temp1=51.00 degC (TC0H CPU0 Heatsink)
> hw.sensors.asmc0.temp2=58.00 degC (TC0P CPU0 Proximity)
> hw.sensors.asmc0.temp3=78.00 degC (TG0D GPU0 Diode)
> hw.sensors.asmc0.temp4=73.00 degC (TG0H GPU0 Heatsink)
> hw.sensors.asmc0.temp5=49.00 degC (TL0P LCD Proximity)
> hw.sensors.asmc0.temp6=51.00 degC (TO0P Optical Drive)
> hw.sensors.asmc0.temp7=67.00 degC (Tm0P Memory Controller)
> hw.sensors.asmc0.fan0=997 RPM (ODD)
> hw.sensors.asmc0.fan1=1099 RPM (HDD)
> hw.sensors.asmc0.fan2=1198 RPM (CPU)
> hw.sensors.asmc0.illuminance0=4078.00 lx (left)
> $
> 
> OpenBSD 5.7-stable (ASMC) #1: Wed Sep 30 16:46:45 CEST 2015
> mart...@martijn.office.rootnet.nl:/usr/src/sys/arch/amd64/compile/ASMC
> RTC BIOS diagnostic error d1<clock_battery,ROM_cksum,memory_size>
> real mem = 4253237248 (4056MB)
> avail mem = 4136079360 (3944MB)
> mpath0 at root
> scsibus0 at mpath0: 256 targets
> mainbus0 at root
> bios0 at mainbus0: SMBIOS rev. 2.4 @ 0xe (61 entries)
> bios0: vendor Apple Inc. version "IM121.88Z.0047.B1F.1201241648" date
> 01/24/12
> bios0: Apple Inc. iMac12,1
> acpi0 at bios0: rev 2
> acpi0: sleep states S0 S3 S4 S5
> acpi0: tables DSDT FACP HPET APIC SBST ECDT SSDT SSDT SSDT SSDT SSDT SSDT
> SSDT SSDT MCFG SSDT SSDT SSDT
> acpi0: wakeup devices P0P2(S4) GFX0(S4) EC__(S4) HDEF(S4) GIGE(S4) RP01(S4)
> ARPT(S4) RP02(S4) RP03(S4) RP05(S4) EHC1(S3) EHC2(S3)
> acpitimer0 at acpi0: 3579545 Hz, 24 bits
> acpihpet0 at acpi0: 14318179 Hz
> acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
> cpu0 at mainbus0: apid 0 (boot processor)
> cpu0: Intel(R) Core(TM) i5-2400S CPU @ 2.50GHz, 2500.36 MHz
> cpu0: 
> FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,LONG,LAHF,PERF,ITSC
> cpu0: 256KB 64b/line 8-way L2 cache
> cpu0: smt 0, core 0, package 0
> mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
> cpu0: apic clock running at 100MHz
> cpu0: mwait min=64, max=64, C-substates=0.2.1.1.0, IBE
> cpu1 at mainbus0: apid 2 (application processor)
> cpu1: Intel(R) Core(TM) i5-2400S CPU @ 2.50GHz, 2500.03 MHz
> cpu1: 
> FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,LONG,LAHF,PERF,ITSC
> 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) Core(TM) i5-2400S CPU @ 2.50GHz, 2500.03 MHz
> cpu2: 
> FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SS

asmc(4) new driver for Apple SMC

2015-09-26 Thread Joerg Jung
Hi,

please find below a diff which adds a new driver called asmc(4) for the
Apple System Management Controller (SMC), as found in Intel based Apple
devices.

A similar driver is available in Linux[1], FreeBSD[2], and NetBSD[3].
The diff below was motivated by an earlier attempt[4] of Volker Nowarra,
but is completely written from scratch by myself.

The asmc(4) driver exposes several temperature, fan, light, and
acceleration sensor values through sysctl(8). The dmesg on my
MacBook Air looks like this with the diff applied:

asmc0 at isa0 port 0x300/32
asmc0: rev 1.73f573, 362 keys, 21 temperatures, 1 fan, 2 lights, kbdled

... and sysctl(8) shows the following new sensors:

hw.sensors.asmc0.temp0=33.00 degC (TB0T Enclosure Bottom)
hw.sensors.asmc0.temp1=33.00 degC (TB1T Enclosure Bottom 2)
hw.sensors.asmc0.temp2=31.00 degC (TB2T Enclosure Bottom 3)
hw.sensors.asmc0.temp3=65.00 degC (TC0C)
hw.sensors.asmc0.temp4=65.00 degC (TC0D CPU0 Die Core)
hw.sensors.asmc0.temp5=65.00 degC (TC0E)
hw.sensors.asmc0.temp6=65.00 degC (TC0F)
hw.sensors.asmc0.temp7=59.00 degC (TC0P CPU0 Proximity)
hw.sensors.asmc0.temp8=66.00 degC (TC1C)
hw.sensors.asmc0.temp9=64.00 degC (TC2C)
hw.sensors.asmc0.temp10=65.00 degC (TCGC)
hw.sensors.asmc0.temp11=63.00 degC (TCSA)
hw.sensors.asmc0.temp12=40.00 degC (THSP)
hw.sensors.asmc0.temp13=55.00 degC (TM0P Mem Bank A1)
hw.sensors.asmc0.temp14=68.00 degC (TPCD)
hw.sensors.asmc0.temp15=52.00 degC (Ta0P)
hw.sensors.asmc0.temp16=40.00 degC (Th1H Main Heatsink B)
hw.sensors.asmc0.temp17=45.00 degC (Tm0P Memory Controller)
hw.sensors.asmc0.temp18=53.00 degC (Tm1P)
hw.sensors.asmc0.temp19=30.00 degC (Ts0P)
hw.sensors.asmc0.temp20=39.00 degC (Ts0S)
hw.sensors.asmc0.fan0=1994 RPM (Exhaust)
hw.sensors.asmc0.illuminance0=23.00 lx (left)

Also, the keyboard backlight is enabled, if available.

A few notes on the implementation: For the sake of simplicity, the
available SMC index command is NOT used to iterate hundreds of keys.
Instead a list of static keys is used.  Moreover, the status port is
queried after readings to find out if keys are available.  The timeouts
for the SMC are high and the chip seems to be slow. Hundreds of keys
might need to be checked, thus the way of using the static list and
status port is considered much faster then a dynamic index search.

For example, a MacPro might have about 50 temperature sensors and thus
may take several seconds (maybe minutes) to initialize things.

Please see the caveats section in the man page found within the diff for
further details (I consider this my todo list).

If you run OpenBSD on Apple Intel HW, e.g. MacBook {Air,Pro}, iMac,
MacPro, or MacMini, please give this diff a try.

As usual, tests, comments, suggestions, flames, and OKs are welcome.

Thanks,
Regards,
Joerg

[1] https://github.com/torvalds/linux/blob/master/drivers/hwmon/applesmc.c
[2] https://svnweb.freebsd.org/base/head/sys/dev/asmc/
[3] http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/ic/apple_smc.c
[4] http://marc.info/?l=openbsd-tech=139135064628383=2


Index: share/man/man4/asmc.4
===
RCS file: share/man/man4/asmc.4
diff -N share/man/man4/asmc.4
--- /dev/null   1 Jan 1970 00:00:00 -
+++ share/man/man4/asmc.4   26 Sep 2015 12:09:59 -
@@ -0,0 +1,62 @@
+.\"$OpenBSD: $
+.\"
+.\" Copyright (c) 2015 Joerg Jung <j...@openbsd.org>
+.\"
+.\" Permission to use, copy, modify, and distribute this software for any
+.\" purpose with or without fee is hereby granted, provided that the above
+.\" copyright notice and this permission notice appear in all copies.
+.\"
+.\" THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+.\" WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+.\" MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+.\" ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+.\" WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+.\"
+.Dd $Mdocdate: August 11 2015 $
+.Dt ASMC 4
+.Os
+.Sh NAME
+.Nm asmc
+.Nd Apple System Management Controller (SMC)
+.Sh SYNOPSIS
+.Cd "asmc0 at isa? port 0x300"
+.Sh DESCRIPTION
+The
+.Nm
+driver provides support for the Apple System Management Controller (SMC), as
+found in Intel based Apple devices.
+.Pp
+The driver possesses a collection of temperature, fan, light, and acceleration
+sensor values which are made available through the
+.Xr sysctl 8
+interface.
+.Pp
+If available, the keyboard backlight is enabled by the driver.
+.Sh SEE ALSO
+.Xr intro 4 ,
+.Xr isa 4 ,
+.Xr sensorsd 8 ,
+.Xr sysctl 8
+.Sh HISTORY
+The
+.Nm
+driver first appeared in
+.Ox 5.9 .
+.Sh AUTHORS
+.An -nosplit
+The
+.Nm
+driver was written by
+.An Joerg Ju

Re: [patch] httpd: fcgi/PATH_INFO not handled correctly

2015-09-05 Thread Joerg Jung
On Sun, Aug 30, 2015 at 08:40:57PM +0200, Joerg Jung wrote:
> On Wed, Aug 26, 2015 at 08:23:22PM +0200, Denis Fondras wrote:
> > Hello,
> > 
> > While using httpd together uwsgi and Flask, I noticed that GET requests to /
> > returned 404. The same setup with nginx was returning 200.
> > 
> > The culprit is that PATH_INFO is not set when REQUEST_URI is /.
> > The following patch correctly set PATH_INFO in every case.
> 
> Yes, I think your proposed diff is right.
> I would like to see this committed, any dev willing to give OK?

Ping, anyone?

> Thanks,
> Regards,
> Joerg
> 
>  
> > Index: httpd.c
> > ===
> > RCS file: /cvs/src/usr.sbin/httpd/httpd.c,v
> > retrieving revision 1.39
> > diff -u -p -r1.39 httpd.c
> > --- httpd.c 20 Aug 2015 13:00:23 -  1.39
> > +++ httpd.c 26 Aug 2015 18:12:34 -
> > @@ -695,7 +695,7 @@ path_info(char *path)
> >  
> > for (p = end; p > start; p--) {
> > /* Scan every path component from the end and at each '/' */
> > -   if (p < end && *p != '/')
> > +   if (p <= end && *p != '/')
> > continue;
> >  
> > /* Temporarily cut the path component out */
> > 



Re: PF ignores block action when rule contains route-to/dup-to action

2015-09-01 Thread Joerg Jung

> On 01 Sep 2015, at 14:31, Alexandr Nedvedicky 
>  wrote:
> 
>>> As a side effect the patch breaks block rules with dup-to action. dup-to
>>> action as a part of block rule might make some sense... So if there is
>>> someone, who really needs block ... dup-to he should opt for equivalent
>>> rule using pass ... route-to 
>>> 
>>> Also there is one more question:
>>> 
>>>   shall we implement similar sanity checks for nat-to/rdr-to/... actions?
>> 
>> IMHO, yes that would make sense.
>> 
> 
> I'll try to keep it on my todo list...
> 
>> Some bike shedding inline below, apart from that, ok jung@
> 
> I love bike shedding, so let's go an pick up some colors ;-)

:)

>> 
>>> -   if (r->action == PF_MATCH) {
>>> +   /* 
>>> +* Basic rule sanity check.
>>> +*/
>> 
>> A single line comment is enough here, isn’t it?
>> 
> 
> O.K. new patch has single line comment.
> 
>>> +   switch (r->action) {
>>> +   case PF_MATCH:
>>> if (r->divert.port) {
>>> yyerror("divert is not supported on match rules");
>>> problems++;
>>> @@ -4016,6 +4019,15 @@ rule_consistent(struct pf_rule *r, int a
>>> yyerror("af-to is not supported on match rules");
>>> problems++;
>>> }
>>> +   break;
>>> +   case PF_DROP:
>>> +   if (r->rt) {
>>> +   yyerror("route-to, reply-to and dup-to "
>>> +  "must not be used on block rules”);
>> 
>> The other error messages say “is not supported” instead of “must no be used”.
>> I do not care which wording you chose, but maybe take the chance and unify 
>> it, 
>> to be more consistent here? 
>> 
> 
> the question is which consistency do you want.

As written above: I do not really care.

> The patch does not show them,
> but there are two options, actually three. Let me show the current code 
> without
> patch applied:
> 
>4000 /* match rules rules */
>4001 if (r->action == PF_MATCH) {
>4002 if (r->divert.port) {
>4003 yyerror("divert is not supported on match 
> rules");
>4004 problems++;
>4005 }
>4006 if (r->divert_packet.port) {
>4007 yyerror("divert is not supported on match 
> rules");
>4008 problems++;
>4009 }
>4010 if (r->rt) {
>4011 yyerror("route-to, reply-to and dup-to "
>4012"must not be used on match rules");
>4013 problems++;
>4014 }
>4015 if (r->rule_flag & PFRULE_AFTO) {
>4016 yyerror("af-to is not supported on match 
> rules");
>4017 problems++;
>4018 }
>4019 }
> 
> as you can see there are two colors to chose from:
> 
>color A:
>   ... is not supported on ... rules (used at 4003, 4007, 4016
> 
>color B:
>   ... must not be used on match rules (used at line 4911)
> 
> we have three options:
> 
>1) leave it as it is (both colors will be used)
> 
>2) use color A
> 
>3) use color B
> 
> IMO consistency is good here. I prefer color A as it sounds more polite. 

Yes, good choice. :)

> updated patch is further below.

ok jung@

> regards
> sasha
> 
> 8<---8<---8<--8<
> 
> Index: sbin/pfctl/parse.y
> ===
> RCS file: /cvs/src/sbin/pfctl/parse.y,v
> retrieving revision 1.648
> diff -u -p -r1.648 parse.y
> --- sbin/pfctl/parse.y21 Apr 2015 16:34:59 -  1.648
> +++ sbin/pfctl/parse.y1 Sep 2015 12:29:41 -
> @@ -3997,8 +3997,9 @@ rule_consistent(struct pf_rule *r, int a
>   problems++;
>   }
> 
> - /* match rules rules */
> - if (r->action == PF_MATCH) {
> + /* Basic rule sanity check. */
> + switch (r->action) {
> + case PF_MATCH:
>   if (r->divert.port) {
>   yyerror("divert is not supported on match rules");
>   problems++;
> @@ -4009,13 +4010,22 @@ rule_consistent(struct pf_rule *r, int a
>   }
>   if (r->rt) {
>   yyerror("route-to, reply-to and dup-to "
> -"must not be used on match rules");
> +"are not supported on match rules");
>   problems++;
>   }
>   if (r->rule_flag & PFRULE_AFTO) {
>   yyerror("af-to is not supported on match rules");
>   problems++;
>   }
> + break;
> + case PF_DROP:
> + if (r->rt) {
> + yyerror("route-to, reply-to and dup-to "
> +   

Re: [patch] httpd: fcgi/PATH_INFO not handled correctly

2015-08-30 Thread Joerg Jung
On Wed, Aug 26, 2015 at 08:23:22PM +0200, Denis Fondras wrote:
 Hello,
 
 While using httpd together uwsgi and Flask, I noticed that GET requests to /
 returned 404. The same setup with nginx was returning 200.
 
 The culprit is that PATH_INFO is not set when REQUEST_URI is /.
 The following patch correctly set PATH_INFO in every case.

Yes, I think your proposed diff is right.
I would like to see this committed, any dev willing to give OK?

Thanks,
Regards,
Joerg

 
 Denis
 
 
 Index: httpd.c
 ===
 RCS file: /cvs/src/usr.sbin/httpd/httpd.c,v
 retrieving revision 1.39
 diff -u -p -r1.39 httpd.c
 --- httpd.c   20 Aug 2015 13:00:23 -  1.39
 +++ httpd.c   26 Aug 2015 18:12:34 -
 @@ -695,7 +695,7 @@ path_info(char *path)
  
   for (p = end; p  start; p--) {
   /* Scan every path component from the end and at each '/' */
 - if (p  end  *p != '/')
 + if (p = end  *p != '/')
   continue;
  
   /* Temporarily cut the path component out */
 



Re: [PATCH] PF: cksum modification refactor [1/24]

2015-08-30 Thread Joerg Jung
On Sun, Aug 30, 2015 at 06:22:14PM +1200, Richard Procter wrote:
 * constify local aliases
 
   - enables compilation to verify they are never reassigned
 
 ok?

Your next patch [2/24] does remove this line, so no need for this one, 
right? 
 
 to apply: 
 $ cd /usr/src/sys
 $ cat - | patch
 
 Index: net/pf.c
 ===
 --- net.orig/pf.c
 +++ net/pf.c
 @@ -4490,7 +4490,7 @@ int
  pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **state,
  u_short *reason)
  {
 - struct pf_addr  *saddr = pd-src, *daddr = pd-dst;
 + struct pf_addr  *const saddr = pd-src, *const daddr = pd-dst;
   u_int16_tvirtual_id, virtual_type;
   u_int8_t icmptype;
   int  icmp_dir, iidx, ret, copyback = 0;
 



Re: [PATCH] PF: cksum modification refactor [2/24]

2015-08-30 Thread Joerg Jung
On Sun, Aug 30, 2015 at 06:22:30PM +1200, Richard Procter wrote:
 * Remove local aliases saddr, daddr
 
   - eases refactoring later on

I fail to see how yet, but ... 

[...]
 
 ok?

... with the assumption that the above statement will
become true in the bigger picture, ok jung@

 to apply: 
 $ cd /usr/src/sys
 $ cat - | patch
 
 Index: net/pf.c
 ===
 --- net.orig/pf.c
 +++ net/pf.c
 @@ -4490,7 +4490,6 @@ int
  pf_test_state_icmp(struct pf_pdesc *pd, struct pf_state **state,
  u_short *reason)
  {
 - struct pf_addr  *const saddr = pd-src, *const daddr = pd-dst;
   u_int16_tvirtual_id, virtual_type;
   u_int8_t icmptype;
   int  icmp_dir, iidx, ret, copyback = 0;
 @@ -4560,12 +4559,12 @@ pf_test_state_icmp(struct pf_pdesc *pd,
  #endif /* INET6 */
   if (!afto  PF_ANEQ(pd-src,
   nk-addr[sidx], AF_INET))
 - pf_change_a(pd, saddr-v4.s_addr,
 + pf_change_a(pd, pd-src-v4.s_addr,
   nk-addr[sidx].v4.s_addr);
  
   if (!afto  PF_ANEQ(pd-dst,
   nk-addr[didx], AF_INET)) {
 - pf_change_a(pd, daddr-v4.s_addr,
 + pf_change_a(pd, pd-dst-v4.s_addr,
   nk-addr[didx].v4.s_addr);
   pd-destchg = 1;
   }
 @@ -4592,12 +4591,12 @@ pf_test_state_icmp(struct pf_pdesc *pd,
   }
   if (!afto  PF_ANEQ(pd-src,
   nk-addr[sidx], AF_INET6))
 - pf_change_a6(pd, saddr,
 + pf_change_a6(pd, pd-src,
   nk-addr[sidx]);
  
   if (!afto  PF_ANEQ(pd-dst,
   nk-addr[didx], AF_INET6)) {
 - pf_change_a6(pd, daddr,
 + pf_change_a6(pd, pd-dst,
   nk-addr[didx]);
   pd-destchg = 1;
   }
 @@ -4845,7 +4844,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
   nk-addr[pd2.sidx], pd2.af) ||
   nk-port[pd2.sidx] != th.th_sport)
   pf_change_icmp(pd, pd2.src,
 - th.th_sport, daddr,
 + th.th_sport, pd-dst,
   nk-addr[pd2.sidx],
   nk-port[pd2.sidx], pd2.af);
  
 @@ -4858,7 +4857,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
   nk-addr[pd2.didx], pd2.af) ||
   nk-port[pd2.didx] != th.th_dport)
   pf_change_icmp(pd, pd2.dst,
 - th.th_dport, saddr,
 + th.th_dport, pd-src,
   nk-addr[pd2.didx],
   nk-port[pd2.didx], pd2.af);
   copyback = 1;
 @@ -4961,7 +4960,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
   nk-addr[pd2.sidx], pd2.af) ||
   nk-port[pd2.sidx] != uh.uh_sport)
   pf_change_icmp(pd, pd2.src,
 - uh.uh_sport, daddr,
 + uh.uh_sport, pd-dst,
   nk-addr[pd2.sidx],
   nk-port[pd2.sidx], pd2.af);
  
 @@ -4974,7 +4973,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
   nk-addr[pd2.didx], pd2.af) ||
   nk-port[pd2.didx] != uh.uh_dport)
   pf_change_icmp(pd, pd2.dst,
 - uh.uh_dport, saddr,
 + uh.uh_dport, pd-src,
   nk-addr[pd2.didx],
   nk-port[pd2.didx], pd2.af);
  
 @@ -5083,7 +5082,7 @@ pf_test_state_icmp(struct pf_pdesc *pd,
   pf_change_icmp(pd, pd2.src,
   (virtual_type == htons(ICMP_ECHO)) ?
   iih.icmp_id : NULL,
 - daddr, nk-addr[pd2.sidx],
 + pd-dst, nk-addr[pd2.sidx],

Re: [PATCH] ukbd.c cleanup and mba iso support

2015-08-06 Thread Joerg Jung
On Wed, Feb 18, 2015 at 10:33:57PM -0800, William Orr wrote:
 Hey,
 
 Any interest?

I'm interested in this. Your diff looks reasonable, so I applied it and 
it compiled fine, but the less and grave keys are still wrong exchanged
on my MacBookAir4,2. Looks like usbdevs says I have a 
USB_PRODUCT_APPLE_WELLSPRING6_ISO:

   port 2 addr 7: full speed, power 40 mA, config 1, Apple Internal
  Keyboard / Trackpad(0x024d), Apple Inc.(0x05ac), rev 2.09   

So I guess some more case's are required in the diff below?
 
 Thanks,
 William Orr
 
 On 2/4/15 9:37 AM, William Orr wrote:
  Hey,
  
  This implements some of Alexey's comments as well as munging the grave key 
  for
  macbook airs. Tested on a mba with a WELLSPRING ANSI keyboard.
  
  Thanks,
  William Orr
  
  Index: sys/dev/usb/ukbd.c
  ===
  RCS file: /cvs/src/sys/dev/usb/ukbd.c,v
  retrieving revision 1.70
  diff -u -b -w -p -r1.70 ukbd.c
  --- sys/dev/usb/ukbd.c  19 Jan 2015 20:16:10 -  1.70
  +++ sys/dev/usb/ukbd.c  4 Feb 2015 05:18:47 -
  @@ -182,6 +182,11 @@ void   ukbd_gdium_munge(void *, uint8_t *,
   void   ukbd_apple_munge(void *, uint8_t *, u_int);
   void   ukbd_apple_mba_munge(void *, uint8_t *, u_int);
   void   ukbd_apple_iso_munge(void *, uint8_t *, u_int);
  +void   ukbd_apple_iso_mba_munge(void *, uint8_t *, u_int);
  +
  +void ukbd_apple_translate(void *, uint8_t *, u_int,
  +  const struct ukbd_translation *, u_int);
  +
   uint8_tukbd_translate(const struct ukbd_translation *, size_t, 
  uint8_t);
   
   int
  @@ -244,14 +249,16 @@ ukbd_attach(struct device *parent, struc
  case USB_PRODUCT_APPLE_GEYSER_ISO:
  sc-sc_munge = ukbd_apple_iso_munge;
  break;
  -   case USB_PRODUCT_APPLE_WELLSPRING4A_ANSI:
  case USB_PRODUCT_APPLE_WELLSPRING4A_ISO:
  +   case USB_PRODUCT_APPLE_WELLSPRING4_ISO:
  +   case USB_PRODUCT_APPLE_WELLSPRING_ISO:
  +   sc-sc_munge = ukbd_apple_iso_mba_munge;
  +   break;
  +   case USB_PRODUCT_APPLE_WELLSPRING4A_ANSI:
  case USB_PRODUCT_APPLE_WELLSPRING4A_JIS:
  case USB_PRODUCT_APPLE_WELLSPRING4_ANSI:
  -   case USB_PRODUCT_APPLE_WELLSPRING4_ISO:
  case USB_PRODUCT_APPLE_WELLSPRING4_JIS:
  case USB_PRODUCT_APPLE_WELLSPRING_ANSI:
  -   case USB_PRODUCT_APPLE_WELLSPRING_ISO:
  case USB_PRODUCT_APPLE_WELLSPRING_JIS:
  sc-sc_munge = ukbd_apple_mba_munge;
  break;
  @@ -461,12 +468,28 @@ ukbd_translate(const struct ukbd_transla
   }
   
   void
  -ukbd_apple_munge(void *vsc, uint8_t *ibuf, u_int ilen)
  +ukbd_apple_translate(void *vsc, uint8_t *ibuf, u_int ilen,
  +const struct ukbd_translation* trans, u_int tlen)
   {
  struct ukbd_softc *sc = vsc;
  struct hidkbd *kbd = sc-sc_kbd;
  uint8_t *pos, *spos, *epos, xlat;
   
  +   spos = ibuf + kbd-sc_keycodeloc.pos / 8;
  +   epos = spos + kbd-sc_nkeycode;
  +
  +   for (pos = spos; pos != epos; pos++) {
  +   xlat = ukbd_translate(trans, tlen, *pos);
  +   if (xlat != 0)
  +   *pos = xlat;
  +   }
  +}
  +
  +void
  +ukbd_apple_munge(void *vsc, uint8_t *ibuf, u_int ilen)
  +{
  +   struct ukbd_softc *sc = vsc;
  +
  static const struct ukbd_translation apple_fn_trans[] = {
  { 40, 73 }, /* return - insert */
  { 42, 76 }, /* backspace - delete */
  @@ -499,23 +522,14 @@ ukbd_apple_munge(void *vsc, uint8_t *ibu
  if (!hid_get_data(ibuf, ilen, sc-sc_apple_fn))
  return;
   
  -   spos = ibuf + kbd-sc_keycodeloc.pos / 8;
  -   epos = spos + kbd-sc_nkeycode;
  -
  -   for (pos = spos; pos != epos; pos++) {
  -   xlat = ukbd_translate(apple_fn_trans,
  -   nitems(apple_fn_trans), *pos);
  -   if (xlat != 0)
  -   *pos = xlat;
  -   }
  +   ukbd_apple_translate(vsc, ibuf, ilen, apple_fn_trans,
  +nitems(apple_fn_trans));
   }
   
   void
   ukbd_apple_mba_munge(void *vsc, uint8_t *ibuf, u_int ilen)
   {
  struct ukbd_softc *sc = vsc;
  -   struct hidkbd *kbd = sc-sc_kbd;
  -   uint8_t *pos, *spos, *epos, xlat;
   
  static const struct ukbd_translation apple_fn_trans[] = {
  { 40, 73 }, /* return - insert */
  @@ -545,40 +559,34 @@ ukbd_apple_mba_munge(void *vsc, uint8_t 
  if (!hid_get_data(ibuf, ilen, sc-sc_apple_fn))
  return;
   
  -   spos = ibuf + kbd-sc_keycodeloc.pos / 8;
  -   epos = spos + 

Re: [Patch] httpd - don't leak fcgi file descriptors

2015-06-09 Thread Joerg Jung
On Tue, Jun 09, 2015 at 07:46:15AM +, Florian Obser wrote:
 On Mon, Jun 08, 2015 at 09:17:41PM +0200, Claudio Jeker wrote:
  On Mon, Jun 08, 2015 at 09:12:32PM +0200, Joerg Jung wrote:
   On Tue, Jun 02, 2015 at 05:47:47PM +0200, Claudio Jeker wrote:
On Tue, Jun 02, 2015 at 01:50:35PM +0200, Joerg Jung wrote:

  Am 01.06.2015 um 01:25 schrieb Todd Mortimer t...@opennet.ca:
 
  I agree that my patch is more of a workaround, and it would be
  better to track down how it is that the client is being passed to
  server_fcgi with an open socket. I was going this way when I started
  looking at the source, but then I saw that clt-clt_srvevb and
  clt-clt_srvbev get the same treatment (free if not null, then
  reassign) at the same spot in server_fcgi(), and I figured if it
  was good enough for clt_srvevb and clt_srvbev, why not for clt_fd?

 Yes, you are right. I think your proposed diff is correct.
 I would like to commit it, if anyone is willing to give OK.
   
This feels to me more like a workaround. Since what happens is that a
connection is either reused without cleanup or we call the connection
establishment multiple time for the same client.
relayd had a similar issue that I fixed lately. One of the issues is 
that
event callbacks can be called when you don't really expect them to be
called.
   
   Yes, workaround was my first impression as well.  But after staring at
   the code for a while, I think  fixing it in the right way seems not
   trivial and involves several changes.
   
   So, since the diff below is simple and goes along with the style of the
   existing code, AND fixes an actual leak, I would suggest to commit it
   for now, until someone comes up with something better?
  
  Sure. Fine with me. Wondering if the -1 check is needed. IIRC close(-1);
  is save. Anyway you want to add a space after the if.
   
 
 OK for me as well. Please go ahead. I was hoping to look into it and
 find the correct fix but I didn't get around to it. So this
 workaround is better than nothing I guess.

Committed, thanks.

Regards,
Joerg



Re: [Patch] httpd - don't leak fcgi file descriptors

2015-06-08 Thread Joerg Jung
On Tue, Jun 02, 2015 at 05:47:47PM +0200, Claudio Jeker wrote:
 On Tue, Jun 02, 2015 at 01:50:35PM +0200, Joerg Jung wrote:
 
   Am 01.06.2015 um 01:25 schrieb Todd Mortimer t...@opennet.ca:
  
   I agree that my patch is more of a workaround, and it would be
   better to track down how it is that the client is being passed to
   server_fcgi with an open socket. I was going this way when I started
   looking at the source, but then I saw that clt-clt_srvevb and
   clt-clt_srvbev get the same treatment (free if not null, then
   reassign) at the same spot in server_fcgi(), and I figured if it
   was good enough for clt_srvevb and clt_srvbev, why not for clt_fd?
 
  Yes, you are right. I think your proposed diff is correct.
  I would like to commit it, if anyone is willing to give OK.

 This feels to me more like a workaround. Since what happens is that a
 connection is either reused without cleanup or we call the connection
 establishment multiple time for the same client.
 relayd had a similar issue that I fixed lately. One of the issues is that
 event callbacks can be called when you don't really expect them to be
 called.

Yes, workaround was my first impression as well.  But after staring at
the code for a while, I think  fixing it in the right way seems not
trivial and involves several changes.

So, since the diff below is simple and goes along with the style of the
existing code, AND fixes an actual leak, I would suggest to commit it
for now, until someone comes up with something better?

Regards,
Joerg

  
   Index: server_fcgi.c
   ===
   RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
   retrieving revision 1.53
   diff -u -p -u -p -r1.53 server_fcgi.c
   --- server_fcgi.c26 Mar 2015 09:01:51 -1.53
   +++ server_fcgi.c31 May 2015 22:33:54 -
   @@ -31,6 +31,7 @@
   #include stdio.h
   #include time.h
   #include ctype.h
   +#include unistd.h
   #include event.h
  
   #include httpd.h
   @@ -152,6 +153,9 @@ server_fcgi(struct httpd *env, struct cl
  errstr = failed to allocate evbuffer;
  goto fail;
  }
   +
   +if(clt-clt_fd != -1)
   +close(clt-clt_fd);
  
  clt-clt_fd = fd;
  if (clt-clt_srvbev != NULL)
  
 

 --
 :wq Claudio




Re: [Patch] httpd - don't leak fcgi file descriptors

2015-06-02 Thread Joerg Jung

 Am 01.06.2015 um 01:25 schrieb Todd Mortimer t...@opennet.ca:
 
 I agree that my patch is more of a workaround, and it would be
 better to track down how it is that the client is being passed to
 server_fcgi with an open socket. I was going this way when I started
 looking at the source, but then I saw that clt-clt_srvevb and
 clt-clt_srvbev get the same treatment (free if not null, then
 reassign) at the same spot in server_fcgi(), and I figured if it
 was good enough for clt_srvevb and clt_srvbev, why not for clt_fd?

Yes, you are right. I think your proposed diff is correct.
I would like to commit it, if anyone is willing to give OK.

Thanks,
Regards,
Joerg

 
 Index: server_fcgi.c
 ===
 RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
 retrieving revision 1.53
 diff -u -p -u -p -r1.53 server_fcgi.c
 --- server_fcgi.c26 Mar 2015 09:01:51 -1.53
 +++ server_fcgi.c31 May 2015 22:33:54 -
 @@ -31,6 +31,7 @@
 #include stdio.h
 #include time.h
 #include ctype.h
 +#include unistd.h
 #include event.h
 
 #include httpd.h
 @@ -152,6 +153,9 @@ server_fcgi(struct httpd *env, struct cl
errstr = failed to allocate evbuffer;
goto fail;
}
 +
 +if(clt-clt_fd != -1)
 +close(clt-clt_fd);
 
clt-clt_fd = fd;
if (clt-clt_srvbev != NULL)
 



Re: ftp(1) rewrite

2015-06-01 Thread Joerg Jung
On Mon, Jun 01, 2015 at 08:06:38PM +0100, Stuart Henderson wrote:
 On 2015/06/01 10:20, patrick keshishian wrote:
  On 6/1/15, Sunil Nimmagadda su...@nimmagadda.net wrote:
   On Thu, May 21, 2015 at 11:16:09PM -0400, Ted Unangst wrote:
   screw ftp. just make a new util http, that just does http.
 
 Sorry, it's not good enough to replace ftp(1) for system use without
 ftp. Like it or not, ports fetches need FTP and can't really rely on
 installing something for ports to do that.

Yes, but splitting these protocols is good, right?  IMHO, having a clean
and simple http(1) and a (more) clean ftp(1) with the http bits removed
makes sense to me.
 
   + if (gethostname(hostname, sizeof(hostname)) != 0)
   + hostname[0] = '\0';
  
  Pretty sure the Host: header should indicate the remote host,
  not local.
 
 Yep. And watch out to use  the correct hostname for proxies.
 
  But afaik HTTP/1.0 doesn't require that header.
 
 Host: is pretty much required, one particular thing for OpenBSD is that
 fw_update mirrors need it.
 



Re: [Patch] httpd - don't leak fcgi file descriptors

2015-05-31 Thread Joerg Jung
Hi,

 Am 20.05.2015 um 02:06 schrieb Todd Mortimer t...@opennet.ca:
 
 The attached patch fixes a problem I’ve been having with httpd +
 php_fpm + owncloud on 5.7. The patch is against 5.7-release.

Can you try with recent snapshot, and see if issue
still occurs, please?
Development happens in -current.

 After several days running owncloud with httpd, php_fpm started
 complaining about hitting pm.max_children, and top would show a
 bunch of idle php_fpm processes waiting on netio. Eventually httpd
 would start returning error 500 and owncloud would stop working.
 Restarting php_fpm or httpd would temporarily fix the issue. The
 same server with nginx did not have the same problem.
 
 I’ve had this patch running for 5 days now, and php_fpm isnt leaving
 idle processes lying around anymore. I did run with some debugging
 output to verify that clt-clt_fd is sometimes not -1 when it is
 overwritten with the new socket fd.

IMHO your proposed fix is just a workaround.
Instead of 'blindly' close()'ing, better approach is to
figure out where the fd was leaked earlier.

Regards,
Joerg

 Index: server_fcgi.c
 ===
 RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
 retrieving revision 1.52
 diff -u -p -u -p -r1.52 server_fcgi.c
 --- server_fcgi.c   23 Feb 2015 19:22:43 -  1.52
 +++ server_fcgi.c   15 May 2015 22:12:30 -
 @@ -31,6 +31,7 @@
 #include stdio.h
 #include time.h
 #include ctype.h
 +#include unistd.h
 #include event.h
 
 #include httpd.h
 @@ -152,6 +153,9 @@ server_fcgi(struct httpd *env, struct cl
   errstr = failed to allocate evbuffer;
   goto fail;
   }
 +
 +   if (clt-clt_fd != -1)
 +   close(clt-clt_fd);
 
   clt-clt_fd = fd;
   if (clt-clt_srvbev != NULL)
 



Re: implement CLOCK_VIRTUAL and CLOCK_PROF

2014-10-16 Thread Joerg Jung

 Am 16.10.2014 um 06:50 schrieb Philip Guenther guent...@gmail.com:
 
 On Wed, Oct 15, 2014 at 2:08 PM, Todd C. Miller
 todd.mil...@courtesan.com wrote:
 On Wed, 15 Oct 2014 21:53:47 +0200, Alexandre Ratchov wrote:
 On Wed, Oct 15, 2014 at 11:37:26AM -0600, Todd C. Miller wrote:
 Since this came up in another thread.  Trivial implementations of
 CLOCK_VIRTUAL and CLOCK_PROF, modeled after what FreeBSD does.
 
 out of curiousity, what program needs these?
 
 Possibly none, but we currently define CLOCK_VIRTUAL but don't
 actually support it.
 
 IMO we should just delete CLOCK_VIRTUAL from sys/_time.h and clock_gettime(2).

I agree.



Re: Replace LibreSSL times() call

2014-10-15 Thread Joerg Jung

On 14 Oct 2014, at 22:08, Jonas 'Sortie' Termansen sor...@maxsi.org wrote:

 I noticed libressl's apps.c is using times(3), which is among the functions I 
 am
 aggressively deprecating in my personal system. This patch switches it to use
 the clock_gettime and getrusage instead. I pondered using CLOCK_VIRTUAL rather
 than getrusage, but it turned out to be not be implemented and not portable.
 
 Unfortunately, OS X doesn't have clock_gettime, so the portable version will
 have to add back a times call as a fallback, or perhaps use gettimeofday (but
 this doesn't have the proper time-doesn't-go-backwards semantics).

The OS X specific mach_timebase_info() and mach_absolute_time() 
are may be more appropriate as fallback.

 I didn't use the useful, but non-standard timespecsub and TIMEVAL_TO_TIMESPEC
 macros from sys/time.h to make things easier for the portable version.
 Alternatively they could be used and a fallback implementation can be added to
 the libressl time.h wrapper header.

IMHO the macros are the better choice. 

Regards,
Joerg

 --- libressl-2.1.0/apps/apps.c2014-10-11 18:58:11.0 +0200
 +++ libssl/apps/apps.c2014-10-14 21:31:44.827167386 +0200
 @@ -126,7 +126,6 @@
 
 #include sys/types.h
 #include sys/stat.h
 -#include sys/times.h
 
 #include ctype.h
 #include errno.h
 @@ -135,6 +134,7 @@
 #include limits.h
 #include string.h
 #include strings.h
 +#include time.h
 #include unistd.h
 
 #include apps.h
 @@ -2203,25 +2203,40 @@
 #endif
 /* !OPENSSL_NO_TLSEXT  !OPENSSL_NO_NEXTPROTONEG */
 
 +static struct timespec ts_elapsed(struct timespec a, struct timespec b)
 +{
 + a.tv_sec -= b.tv_sec;
 + a.tv_nsec -= b.tv_nsec;
 + if ( a.tv_nsec  0 )
 + {
 + a.tv_nsec += 10L;
 + a.tv_sec -= 1;
 + }
 + return a;
 +}
 +
 double
 app_tminterval(int stop, int usertime)
 {
 - double ret = 0;
 - struct tms rus;
 - clock_t now = times(rus);
 - static clock_t tmstart;
 + static struct timespec start_ts;
 + struct timespec now_ts;
 
 - if (usertime)
 - now = rus.tms_utime;
 + if (usertime) {
 + struct rusage ru;
 + getrusage(RUSAGE_SELF, ru);
 + now_ts.tv_sec = ru.ru_utime.tv_sec;
 + now_ts.tv_nsec = ru.ru_utime.tv_usec * 1000L;
 + } else {
 + clock_gettime(CLOCK_MONOTONIC, now_ts);
 + }
 
 - if (stop == TM_START)
 - tmstart = now;
 - else {
 - long int tck = sysconf(_SC_CLK_TCK);
 - ret = (now - tmstart) / (double) tck;
 + if (stop == TM_START) {
 + start_ts = now_ts;
 + return 0.0;
   }
 
 - return (ret);
 + struct timespec elapsed_ts = ts_elapsed(now_ts, start_ts);
 + return (double) elapsed_ts.tv_sec + (double) elapsed_ts.tv_nsec / 1E9;
 }
 
 int
 




Re: yacc spacing

2014-02-21 Thread Joerg Jung

Am 21.02.2014 um 03:54 schrieb Ted Unangst t...@tedunangst.com:

 While I was poking around yacc, I noticed that skeleton.c used a
 handrolled fputs instead of calling the function. Fixing that, I
 noticed the code wasn't indented nicely. Fixing that, I figured if
 I've already messed with output.c, I should indent that file too.
 
 Which brings us this. The functional change to skeleton.c is all the
 way at the bottom. Otherwise it mostly replaces four spaces with a
 tab, except where it replaces two spaces with a tab, except where it
 replaces some other amount of whitespace.

[...]

 void
 write_section(char *section[])
 {
 -int c;
 -int i;
 -char *s;
 -FILE *f;
 -
 -f = code_file;
 -for (i = 0; (s = section[i]); ++i)
 -{
 - ++outline;
 - while ((c = *s))
 - {
 - putc(c, f);
 - ++s;
 + int c;

You do not need c here anymore, right?

 + int i;
 + char *s;
 + FILE *f;
 +
 + f = code_file;

Why not shorten to FILE *f = code_file; ?

 + for (i = 0; (s = section[i]); ++i) {
 + ++outline;
 + fputs(s, f);
 + putc('\n', f);

Why this local assignment to f anyway and not 
just fputs(s, code_file) and putc(\n', code_file)?

   }
 - putc('\n', f);
 -}
 }

Otherwise, ok jung@



Re: fail to boot snapshot 5.5 on MBPro8,2

2014-02-06 Thread Joerg Jung
Hi,

Am 05.02.2014 um 15:08 schrieb Sven-Volker Nowarra peb.nowa...@bluewin.ch:

 I tried to install 5.5 snapshots from 2. Feb and 3. Feb onto my laptop 
 MacBook Pro 8,2 - both failed. Then used an older snapshot from 
 spacehopper.org/mirrmon, which claimed to be 12 days old. Failed as well. 
 
 MacBook Pro runs perfectly well with 5.4, and 5.5 bsd.rd lets me go into the 
 installation. After a reboot (bsd.mp), the system hangs after the line where 
 the disk should be mounted, and the nvram/clock message normally appears 
 (actually before going into userland). The screen then turns shortly but 
 steady into white/grey, so the blue kernel lines can't be read anymore.
 
 I then tried to boot with bsd -c, with an external USB keyboard - but it 
 hangs as well (as such no difference to the internal keyboard). Looks like I 
 have two prompts (underlines) on the screen, that are flickering.

I see the same issue with my MacBook Air 4,2

 With this status I cannot provide any dmesg:-(

You may at least provide the dmesg from the working 5.4 bsd and the snapshot 
bsd.rd.

Regards,
Joerg

 The first line after boot -c would show the booting hd0a:bsd: 
 7635180+1660460+1097336... line, and then entry point at  The kernels 
 blue lines would say:
 
 kbc: cmd word write error
 [ using 926384 bytes of bsd ELF symbol table ]
 Copyright (c) 1982, 1986, 1989, 1991, 1993
  The Regents of the University ...
 
 OpenBSD 5.5-beta (GENERIC.MP) #284: Mon Feb  3 07:57:32 MST 2014
  t...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
 RTC BIOS diagnostic error 
 ffclock_battery,ROM_cksum,config_unit,memory_size,fixed_disk,invalid_time
 real mem = 4185079808 (3991MB)
 avail mem = 4065452032 (3877MB)
 User Kernel Config
 UKC
 
 The errors on RTC BIOS line also appears in the fully functional 5.4 
 version. But I couldn't see the line kbc: cmd word write error in other 
 dmesgs on OpenBSD-tech.
 
 As mentioned, no dmesg possible, anyone else with MBPro issue?
 
 Any idea were to get old snapshots? 
 I could try to build the kernel from scratch, I successfully built 5.4, and 
 building with -current shows same behaviour as snapshots. Any ideas where to 
 go from here?
 (No serial port obviously).
 
 Volker
 




Re: Apple SMC chip - driver to read sensors and temperatures

2014-02-02 Thread Joerg Jung
Hi,

On Fri, Jan 24, 2014 at 11:00:32PM +, peb.nowa...@bluewin.ch wrote:
 I  was working on the asmc kernel driver for the Macbooks. I have a 
 MBPro8.2 (mid 2011), and came to get the driver installed, reading all 
 sensors and temperatures into the hw.sensors framework. 
 See
  attached diffs. Beginning line 30 I describe the ideas for the driver. 
 Further information available,  also a todo list. Thx to Joshua@, who 
 helped me to create the diffs below, and provided input for the todo 
 list. Looking forward to get some feedback, especially on other Apple 
 hardware.

I tested this on my MacBook Air 4,2 and got the following in dmesg:

ASMC_MATCH 
 BIOS: vendor Apple Inc., product MacBookAir4,2
 ** ERROR: no model found for MacBookAir4,2
 
Hence, I added blindly the following asmc_model line to your diff: 
+   { MacBookAir4,2, 3, 0,  0, 63, 511, 10 }, 

With this line I got the following in sysctl output:

hw.sensors.applesmc.fan0=1992 RPM (fan0_speed)
hw.sensors.applesmc.fan1=0 RPM (fan1_speed), UNKNOWN
hw.sensors.applesmc.fan2=2000 RPM (fan0_targetspeed)
hw.sensors.applesmc.fan3=0 RPM (fan1_targetspeed), UNKNOWN
hw.sensors.applesmc.fan4=0 RPM (fan0_safespeed), UNKNOWN
hw.sensors.applesmc.fan5=0 RPM (fan1_safespeed), UNKNOWN
hw.sensors.applesmc.fan6=2000 RPM (fan0_minspeed)
hw.sensors.applesmc.fan7=0 RPM (fan1_minspeed), UNKNOWN
hw.sensors.applesmc.fan8=6500 RPM (fan0_maxspeed)
hw.sensors.applesmc.fan9=0 RPM (fan1_maxspeed), UNKNOWN
hw.sensors.applesmc.raw0=36 (Enclosure Bottom)
hw.sensors.applesmc.raw1=76 (CPU Temperature Diode)
hw.sensors.applesmc.illuminance0=0.00 lx (lightright)
hw.sensors.applesmc.illuminance1=0.00 lx (lightleft)
hw.sensors.applesmc.acceleration0=0. m/s^2 (sms_x), UNKNOWN
hw.sensors.applesmc.acceleration1=0. m/s^2 (sms_y), UNKNOWN
hw.sensors.applesmc.acceleration2=0. m/s^2 (sms_z), UNKNOWN

Since, it works only partially, I guess the asmc_model line requires 
further tweaking? Anything else I can test? 
Whole dmesg follows below.

Thanks,
Regards,
Joerg

OpenBSD 5.5-beta (GENERIC.MP) #1: Sun Feb  2 13:14:43 CET 2014
r...@marvin.hq.umaxx.net:/usr/src_apple/sys/arch/amd64/compile/GENERIC.MP
RTC BIOS diagnostic error cfclock_battery,ROM_cksum,fixed_disk,invalid_time
real mem = 4185079808 (3991MB)
avail mem = 4065439744 (3877MB)
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.4 @ 0xe (53 entries)
bios0: vendor Apple Inc. version MBA41.88Z.0077.B0F.1201241549 date 01/24/2012
bios0: Apple Inc. MacBookAir4,2
acpi0 at bios0: rev 2
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP HPET APIC SBST ECDT SSDT SSDT SSDT SSDT SSDT SSDT SSDT 
SSDT MCFG SSDT SSDT SSDT
acpi0: wakeup devices P0P2(S4) EC__(S4) HDEF(S4) ARPT(S4) RP02(S4) EHC1(S3) 
EHC2(S3) ADP1(S4) LID0(S4)
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpihpet0 at acpi0: 14318179 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Core(TM) i7-2677M CPU @ 1.80GHz, 1800.30 MHz
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,LONG,LAHF,PERF,ITSC
cpu0: 256KB 64b/line 8-way L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 100MHz
cpu0: mwait min=64, max=64, C-substates=0.2.1.1.2, IBE
cpu1 at mainbus0: apid 2 (application processor)
cpu1: Intel(R) Core(TM) i7-2677M CPU @ 1.80GHz, 1800.02 MHz
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,LONG,LAHF,PERF,ITSC
cpu1: 256KB 64b/line 8-way L2 cache
cpu1: smt 0, core 1, package 0
cpu2 at mainbus0: apid 1 (application processor)
cpu2: Intel(R) Core(TM) i7-2677M CPU @ 1.80GHz, 1800.02 MHz
cpu2: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,LONG,LAHF,PERF,ITSC
cpu2: 256KB 64b/line 8-way L2 cache
cpu2: smt 1, core 0, package 0
cpu3 at mainbus0: apid 3 (application processor)
cpu3: Intel(R) Core(TM) i7-2677M CPU @ 1.80GHz, 1800.02 MHz
cpu3: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,LONG,LAHF,PERF,ITSC
cpu3: 256KB 64b/line 8-way L2 cache
cpu3: smt 1, core 1, package 0
ioapic0 at mainbus0: apid 2 pa 0xfec0, version 20, 24 pins
ioapic0: misconfigured as apic 0, remapped to apid 2
acpiec0 at acpi0
acpimcfg0 at 

Re: security(8) check maildir as well as mailbox permissions

2013-12-20 Thread Joerg Jung

Am 20.12.2013 um 08:48 schrieb David Gwynne da...@gwynne.id.au:

 On 20 Dec 2013, at 2:56 am, Alexander Hall alexan...@beard.se wrote:
 
 Henning Brauer lists-openbsdt...@bsws.de wrote:
 * Craig R. Skinner skin...@britvault.co.uk [2013-12-19 10:18]:
 On 2013-12-18 Wed 20:48 PM |, J??r??mie Courr??ges-Anglas wrote:
 skin...@britvault.co.uk (Craig R. Skinner) writes:
 On 2013-12-18 Wed 15:54 PM |, Stuart Henderson wrote:
 Check the security of /var/mail/dirs similar to
 /var/mail/boxes:
 
 
 Indeed, but security(8) really reflects things in the base OS,
 
 
 smtpd.conf(8)
  deliver to maildir path
  Mail is added to a maildir.  Its location, path, may
  contain format specifiers that are expanded before use
 
 
 Therefore: ... deliver to maildir /var/mail/%{user.username}
 Therefore?  How so?  What's the logic, here?
 THEREFORE software in base can deliver to maildir in /var/mail
 
 THEREFORE software in base can also deliver mail to
 /omgohmymail/pr0n/$uid - does that mean we check it in security?
 
 The question is rather wether Maildirs in /var/mail are a common
 enough setup to warrant a check in security.
 
 I totally agree with Henning here.
 
 That said, I ended up putting my Maildirs in /var/maildir because of this, 
 so I for one wouldn't object.
 
 i also put maildirs in /var/maildir...

Similar discussion, pops up from time to time:
http://marc.info/?l=openbsd-miscm=133422769629575w=2

Quoting sthen@ in the old thread:
/var/mail is intended for user-owned mbox files, I would think
moving your maildirs elsewhere is more sane. I tend to use /mail
for virtual user mailboxes but each to their own :)

IMHO, some standard/best practice directory for maildirs is 
missing in hier(7).

FWIIW, I put mine in /var/vmail but I would move mine to anything 
else to fulfill standard/best practices. 



Re: ssl(8) cert generation instructions

2013-03-07 Thread Joerg Jung

Am 06.03.2013 um 19:23 schrieb Stefan Sperling s...@openbsd.org:

 On Wed, Mar 06, 2013 at 01:05:16PM +, Stuart Henderson wrote:
 It's not entirely obvious that -x509 actually means produce a
 csr, self-sign it (defaulting to SHA1), throw away the csr and write
 the cert and this had me stuck for a long time when I wanted to
 play with DSA server certs.
 
 So here's a diff which moves DSA cert generation instructions
 to the same style as RSA where the process is to produce a CSR and
 tell people how to sign it in separate steps. It doesn't take much
 longer and is clearer.
 
 As a bonus there are instructions for ECDSA cert generation.
 
 OK?
 
 I'd like to mention in passing that I got bitten recently
 by the default lifetime limit of just 30 days for certs.
 I created my own CA but could only use it for one month :(

Same happened to me a while ago.

 Perhaps that could be mentioned. Or a -days option could be
 added to the example.

I agree, please mention and add the option.

Regards,
Joerg