[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openbsd-ports
Subject:    Re: PATCH sysutils/conky - enable apm_* vars on amd64
From:       david.l.cantrell () gmail ! com
Date:       2011-02-25 21:11:53
Message-ID: 4d681b19.jHC5MT5cJT7VdB42%david.l.cantrell () gmail ! com
[Download RAW message or body]

Stuart Henderson <stu@spacehopper.org> wrote:

> On 2011/02/24 14:37, david.l.cantrell@gmail.com wrote:
> > Attached are updated and new patches for the sysutils/conky port.  The
> > patches enable the ${apm_adapter}, ${apm_battery_life}, and
> > ${apm_battery_time} variables in conky on amd64.  There is also a patch to
> > the man page explaining these variables are also available on OpenBSD.
> > 
> > I made these patches on the conky port from the OPENBSD_4_8 branch.  I am
> > not sure if current has an updated conky port, but I do know that at least
> > the latest stable upstream conky release does not have these patches.
> > I've submitted these fixes to the upstream maintainers.
>
> it's likely that every single port in the tree has changed
> between OPENBSD_4_8 and OPENBSD_4_9, conky certainly has.

Sure.  I'm unclear as to whether the previous release branches still take
on patches after a new stable release is made.  I only have a single
usable OpenBSD system right now, and that's running 4.8.  I figured the
patches would be useful in at least a "hey, has anyone seen if this still
affects 4.9?" capacity.

I did check the upstream code and 1.8.2 (OPENBSD_4_8 is using 1.7.2) still
has the same OpenBSD support, so I don't think anyone upstream has touched
it.  So my thought there was that even if ports for OPENBSD_4_9 has
updated to a more recent version of conky, I think the problem is still
there.

> >  	dev = obsd_sensors.device;	// FIXME: read more than one device
> >  
> > -	/* for (dev = 0; dev < MAXSENSORDEVICES; dev++) { */
> > +	for (dev = 0; dev < MAXSENSORDEVICES; dev++) {
> >  		mib[2] = dev;
> >  		if (sysctl(mib, 3, &sensordev, &sdlen, NULL, 0) == -1) {
> > -			if (errno != ENOENT) {
> > -				warn("sysctl");
> > -			}
> > -			return;
> > -			// continue;
> > +			if (errno == ENOENT) /* end of sensors */
> > +				return;
> > +			if (errno == ENXIO) /* missing e.g. usb sensor that was unplugged */
> > +				continue;
> >  		}
> >  		for (type = 0; type < SENSOR_MAX_TYPES; type++) {
> >  			mib[3] = type;
> > @@ -496,7 +495,8 @@ void update_obsd_sensors()
> >  				sensor_cnt++;
> >  			}
> >  		}
> > -	/* } */
> > +		break; // FIXME: read more than one device
> > +	}
>
> did you check that multiple devices work properly? istr having
> problems with this when i converted it to the new sensors api.

No, I did not.  The first two hunks of patch-src_openbsd_c were already
there, my additions were the architecture checks in hunks 3, 4, and 5.

> > -#if  defined(__i386) || defined(__x86_64)
> > +#if  defined(__i386) || defined(__amd64)
>
> no need for this, __x86_64 is already defined.
>
> $ arch -s
> amd64
>
> $ cpp -dM /dev/null | grep x86
> #define __x86_64 1
> #define __x86_64__ 1

Fair enough.  I was unsure if this was a style preference or not.  I
decided to go with 'amd64' as that's the arch name and I'd seen a least
several other places where the __amd64 macros are used rather than
__x86_64.

> > +             || defined(__OpenBSD__)) \
> > +             && (defined(i386) || defined(__i386__) || defined(__amd64__))
>
> is it necessary to restrict arch like this? there are others
> which use apm (probably at least zaurus, macppc, longsoon).

Probably not.  It may be better to invert the architecture check because
the list of non-applicable arches may be shorter.  Upstream had it
restricted to i386 already, I just added amd64.

Thanks,

-- 
David Cantrell <david.l.cantrell@gmail.com>
KB1PCX | http://blog.burdell.org/

[prev in list] [next in list] [prev in thread] [next in thread]