Repository: prometheus/procfs
Pull request #178 (2019-06-11 15:40:58)

sysfs: rewrite PowerSupply parsing code without reflect
This is my take on an ergonomic and reflect-free value parsing API. Although the actual amount of code is slightly larger, this code gives us sane, compile-time checking of all data types and functionality.

Updates #177.


Patch:
diff --git a/go.mod b/go.mod
index 8a1b839..b2f8cca 100644
--- a/go.mod
+++ b/go.mod
@@ -1,3 +1,6 @@
 module github.com/prometheus/procfs
 
-require golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4
+require (
+	github.com/google/go-cmp v0.3.0
+	golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4
+)
diff --git a/go.sum b/go.sum
index 7827dd3..db54133 100644
--- a/go.sum
+++ b/go.sum
@@ -1,2 +1,4 @@
+github.com/google/go-cmp v0.3.0 h1:crn/baboCvb5fXaQ0IJ1SGTsTVrWpDsCWC8EGETZijY=
+github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
 golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4 h1:YUO/7uOKsKeq9UokNS62b8FYywz3ker1l1vDZRCRefw=
 golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
diff --git a/internal/util/valueparser.go b/internal/util/valueparser.go
new file mode 100644
index 0000000..4ecf5a7
--- /dev/null
+++ b/internal/util/valueparser.go
@@ -0,0 +1,58 @@
+// Copyright 2019 The Prometheus Authors
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package util
+
+import (
+	"strconv"
+)
+
+// TODO(mdlayher): util packages are an anti-pattern and this should be moved
+// somewhere else that is more focused in the future.
+
+// A ValueParser enables parsing a single string into a variety of data types
+// in a concise and safe way. The Err method must be invoked after invoking
+// any other methods to ensure a value was successfully parsed.
+type ValueParser struct {
+	v   string
+	err error
+}
+
+// NewValueParser creates a ValueParser using the input string.
+func NewValueParser(v string) *ValueParser {
+	return &ValueParser{v: v}
+}
+
+// PInt64 interprets the underlying value as an int64 and returns a pointer to
+// that value.
+func (vp *ValueParser) PInt64() *int64 {
+	if vp.err != nil {
+		return nil
+	}
+
+	// A base value of zero makes ParseInt infer the correct base using the
+	// string's prefix, if any.
+	const base = 0
+	v, err := strconv.ParseInt(vp.v, base, 64)
+	if err != nil {
+		vp.err = err
+		return nil
+	}
+
+	return &v
+}
+
+// Err returns the last error, if any, encountered by the ValueParser.
+func (vp *ValueParser) Err() error {
+	return vp.err
+}
diff --git a/sysfs/class_power_supply.go b/sysfs/class_power_supply.go
index 90c32e5..3fe7dd4 100644
--- a/sysfs/class_power_supply.go
+++ b/sysfs/class_power_supply.go
@@ -18,161 +18,275 @@ package sysfs
 import (
 	"fmt"
 	"io/ioutil"
-	"os"
-	"reflect"
-	"strconv"
-	"strings"
+	"path/filepath"
 
 	"github.com/prometheus/procfs/internal/util"
 )
 
-// PowerSupply contains info from files in /sys/class/power_supply for a single power supply.
+// PowerSupply contains info from files in /sys/class/power_supply for a
+// single power supply.
 type PowerSupply struct {
 	Name                     string // Power Supply Name
-	Authentic                *int64 `fileName:"authentic"`                   // /sys/class/power_suppy/<Name>/authentic
-	Calibrate                *int64 `fileName:"calibrate"`                   // /sys/class/power_suppy/<Name>/calibrate
-	Capacity                 *int64 `fileName:"capacity"`                    // /sys/class/power_suppy/<Name>/capacity
-	CapacityAlertMax         *int64 `fileName:"capacity_alert_max"`          // /sys/class/power_suppy/<Name>/capacity_alert_max
-	CapacityAlertMin         *int64 `fileName:"capacity_alert_min"`          // /sys/class/power_suppy/<Name>/capacity_alert_min
-	CapacityLevel            string `fileName:"capacity_level"`              // /sys/class/power_suppy/<Name>/capacity_level
-	ChargeAvg                *int64 `fileName:"charge_avg"`                  // /sys/class/power_suppy/<Name>/charge_avg
-	ChargeControlLimit       *int64 `fileName:"charge_control_limit"`        // /sys/class/power_suppy/<Name>/charge_control_limit
-	ChargeControlLimitMax    *int64 `fileName:"charge_control_limit_max"`    // /sys/class/power_suppy/<Name>/charge_control_limit_max
-	ChargeCounter            *int64 `fileName:"charge_counter"`              // /sys/class/power_suppy/<Name>/charge_counter
-	ChargeEmpty              *int64 `fileName:"charge_empty"`                // /sys/class/power_suppy/<Name>/charge_empty
-	ChargeEmptyDesign        *int64 `fileName:"charge_empty_design"`         // /sys/class/power_suppy/<Name>/charge_empty_design
-	ChargeFull               *int64 `fileName:"charge_full"`                 // /sys/class/power_suppy/<Name>/charge_full
-	ChargeFullDesign         *int64 `fileName:"charge_full_design"`          // /sys/class/power_suppy/<Name>/charge_full_design
-	ChargeNow                *int64 `fileName:"charge_now"`                  // /sys/class/power_suppy/<Name>/charge_now
-	ChargeTermCurrent        *int64 `fileName:"charge_term_current"`         // /sys/class/power_suppy/<Name>/charge_term_current
-	ChargeType               string `fileName:"charge_type"`                 // /sys/class/power_supply/<Name>/charge_type
-	ConstantChargeCurrent    *int64 `fileName:"constant_charge_current"`     // /sys/class/power_suppy/<Name>/constant_charge_current
-	ConstantChargeCurrentMax *int64 `fileName:"constant_charge_current_max"` // /sys/class/power_suppy/<Name>/constant_charge_current_max
-	ConstantChargeVoltage    *int64 `fileName:"constant_charge_voltage"`     // /sys/class/power_suppy/<Name>/constant_charge_voltage
-	ConstantChargeVoltageMax *int64 `fileName:"constant_charge_voltage_max"` // /sys/class/power_suppy/<Name>/constant_charge_voltage_max
-	CurrentAvg               *int64 `fileName:"current_avg"`                 // /sys/class/power_suppy/<Name>/current_avg
-	CurrentBoot              *int64 `fileName:"current_boot"`                // /sys/class/power_suppy/<Name>/current_boot
-	CurrentMax               *int64 `fileName:"current_max"`                 // /sys/class/power_suppy/<Name>/current_max
-	CurrentNow               *int64 `fileName:"current_now"`                 // /sys/class/power_suppy/<Name>/current_now
-	CycleCount               *int64 `fileName:"cycle_count"`                 // /sys/class/power_suppy/<Name>/cycle_count
-	EnergyAvg                *int64 `fileName:"energy_avg"`                  // /sys/class/power_supply/<Name>/energy_avg
-	EnergyEmpty              *int64 `fileName:"energy_empty"`                // /sys/class/power_suppy/<Name>/energy_empty
-	EnergyEmptyDesign        *int64 `fileName:"energy_empty_design"`         // /sys/class/power_suppy/<Name>/energy_empty_design
-	EnergyFull               *int64 `fileName:"energy_full"`                 // /sys/class/power_suppy/<Name>/energy_full
-	EnergyFullDesign         *int64 `fileName:"energy_full_design"`          // /sys/class/power_suppy/<Name>/energy_full_design
-	EnergyNow                *int64 `fileName:"energy_now"`                  // /sys/class/power_supply/<Name>/energy_now
-	Health                   string `fileName:"health"`                      // /sys/class/power_suppy/<Name>/health
-	InputCurrentLimit        *int64 `fileName:"input_current_limit"`         // /sys/class/power_suppy/<Name>/input_current_limit
-	Manufacturer             string `fileName:"manufacturer"`                // /sys/class/power_suppy/<Name>/manufacturer
-	ModelName                string `fileName:"model_name"`                  // /sys/class/power_suppy/<Name>/model_name
-	Online                   *int64 `fileName:"online"`                      // /sys/class/power_suppy/<Name>/online
-	PowerAvg                 *int64 `fileName:"power_avg"`                   // /sys/class/power_suppy/<Name>/power_avg
-	PowerNow                 *int64 `fileName:"power_now"`                   // /sys/class/power_suppy/<Name>/power_now
-	PrechargeCurrent         *int64 `fileName:"precharge_current"`           // /sys/class/power_suppy/<Name>/precharge_current
-	Present                  *int64 `fileName:"present"`                     // /sys/class/power_suppy/<Name>/present
-	Scope                    string `fileName:"scope"`                       // /sys/class/power_suppy/<Name>/scope
-	SerialNumber             string `fileName:"serial_number"`               // /sys/class/power_suppy/<Name>/serial_number
-	Status                   string `fileName:"status"`                      // /sys/class/power_supply/<Name>/status
-	Technology               string `fileName:"technology"`                  // /sys/class/power_suppy/<Name>/technology
-	Temp                     *int64 `fileName:"temp"`                        // /sys/class/power_suppy/<Name>/temp
-	TempAlertMax             *int64 `fileName:"temp_alert_max"`              // /sys/class/power_suppy/<Name>/temp_alert_max
-	TempAlertMin             *int64 `fileName:"temp_alert_min"`              // /sys/class/power_suppy/<Name>/temp_alert_min
-	TempAmbient              *int64 `fileName:"temp_ambient"`                // /sys/class/power_suppy/<Name>/temp_ambient
-	TempAmbientMax           *int64 `fileName:"temp_ambient_max"`            // /sys/class/power_suppy/<Name>/temp_ambient_max
-	TempAmbientMin           *int64 `fileName:"temp_ambient_min"`            // /sys/class/power_suppy/<Name>/temp_ambient_min
-	TempMax                  *int64 `fileName:"temp_max"`                    // /sys/class/power_suppy/<Name>/temp_max
-	TempMin                  *int64 `fileName:"temp_min"`                    // /sys/class/power_suppy/<Name>/temp_min
-	TimeToEmptyAvg           *int64 `fileName:"time_to_empty_avg"`           // /sys/class/power_suppy/<Name>/time_to_empty_avg
-	TimeToEmptyNow           *int64 `fileName:"time_to_empty_now"`           // /sys/class/power_suppy/<Name>/time_to_empty_now
-	TimeToFullAvg            *int64 `fileName:"time_to_full_avg"`            // /sys/class/power_suppy/<Name>/time_to_full_avg
-	TimeToFullNow            *int64 `fileName:"time_to_full_now"`            // /sys/class/power_suppy/<Name>/time_to_full_now
-	Type                     string `fileName:"type"`                        // /sys/class/power_supply/<Name>/type
-	UsbType                  string `fileName:"usb_type"`                    // /sys/class/power_supply/<Name>/usb_type
-	VoltageAvg               *int64 `fileName:"voltage_avg"`                 // /sys/class/power_supply/<Name>/voltage_avg
-	VoltageBoot              *int64 `fileName:"voltage_boot"`                // /sys/class/power_suppy/<Name>/voltage_boot
-	VoltageMax               *int64 `fileName:"voltage_max"`                 // /sys/class/power_suppy/<Name>/voltage_max
-	VoltageMaxDesign         *int64 `fileName:"voltage_max_design"`          // /sys/class/power_suppy/<Name>/voltage_max_design
-	VoltageMin               *int64 `fileName:"voltage_min"`                 // /sys/class/power_suppy/<Name>/voltage_min
-	VoltageMinDesign         *int64 `fileName:"voltage_min_design"`          // /sys/class/power_suppy/<Name>/voltage_min_design
-	VoltageNow               *int64 `fileName:"voltage_now"`                 // /sys/class/power_supply/<Name>/voltage_now
-	VoltageOCV               *int64 `fileName:"voltage_ocv"`                 // /sys/class/power_suppy/<Name>/voltage_ocv
+	Authentic                *int64 // /sys/class/power_supply/<Name>/authentic
+	Calibrate                *int64 // /sys/class/power_supply/<Name>/calibrate
+	Capacity                 *int64 // /sys/class/power_supply/<Name>/capacity
+	CapacityAlertMax         *int64 // /sys/class/power_supply/<Name>/capacity_alert_max
+	CapacityAlertMin         *int64 // /sys/class/power_supply/<Name>/capacity_alert_min
+	CapacityLevel            string // /sys/class/power_supply/<Name>/capacity_level
+	ChargeAvg                *int64 // /sys/class/power_supply/<Name>/charge_avg
+	ChargeControlLimit       *int64 // /sys/class/power_supply/<Name>/charge_control_limit
+	ChargeControlLimitMax    *int64 // /sys/class/power_supply/<Name>/charge_control_limit_max
+	ChargeCounter            *int64 // /sys/class/power_supply/<Name>/charge_counter
+	ChargeEmpty              *int64 // /sys/class/power_supply/<Name>/charge_empty
+	ChargeEmptyDesign        *int64 // /sys/class/power_supply/<Name>/charge_empty_design
+	ChargeFull               *int64 // /sys/class/power_supply/<Name>/charge_full
+	ChargeFullDesign         *int64 // /sys/class/power_supply/<Name>/charge_full_design
+	ChargeNow                *int64 // /sys/class/power_supply/<Name>/charge_now
+	ChargeTermCurrent        *int64 // /sys/class/power_supply/<Name>/charge_term_current
+	ChargeType               string // /sys/class/power_supply/<Name>/charge_type
+	ConstantChargeCurrent    *int64 // /sys/class/power_supply/<Name>/constant_charge_current
+	ConstantChargeCurrentMax *int64 // /sys/class/power_supply/<Name>/constant_charge_current_max
+	ConstantChargeVoltage    *int64 // /sys/class/power_supply/<Name>/constant_charge_voltage
+	ConstantChargeVoltageMax *int64 // /sys/class/power_supply/<Name>/constant_charge_voltage_max
+	CurrentAvg               *int64 // /sys/class/power_supply/<Name>/current_avg
+	CurrentBoot              *int64 // /sys/class/power_supply/<Name>/current_boot
+	CurrentMax               *int64 // /sys/class/power_supply/<Name>/current_max
+	CurrentNow               *int64 // /sys/class/power_supply/<Name>/current_now
+	CycleCount               *int64 // /sys/class/power_supply/<Name>/cycle_count
+	EnergyAvg                *int64 // /sys/class/power_supply/<Name>/energy_avg
+	EnergyEmpty              *int64 // /sys/class/power_supply/<Name>/energy_empty
+	EnergyEmptyDesign        *int64 // /sys/class/power_supply/<Name>/energy_empty_design
+	EnergyFull               *int64 // /sys/class/power_supply/<Name>/energy_full
+	EnergyFullDesign         *int64 // /sys/class/power_supply/<Name>/energy_full_design
+	EnergyNow                *int64 // /sys/class/power_supply/<Name>/energy_now
+	Health                   string // /sys/class/power_supply/<Name>/health
+	InputCurrentLimit        *int64 // /sys/class/power_supply/<Name>/input_current_limit
+	Manufacturer             string // /sys/class/power_supply/<Name>/manufacturer
+	ModelName                string // /sys/class/power_supply/<Name>/model_name
+	Online                   *int64 // /sys/class/power_supply/<Name>/online
+	PowerAvg                 *int64 // /sys/class/power_supply/<Name>/power_avg
+	PowerNow                 *int64 // /sys/class/power_supply/<Name>/power_now
+	PrechargeCurrent         *int64 // /sys/class/power_supply/<Name>/precharge_current
+	Present                  *int64 // /sys/class/power_supply/<Name>/present
+	Scope                    string // /sys/class/power_supply/<Name>/scope
+	SerialNumber             string // /sys/class/power_supply/<Name>/serial_number
+	Status                   string // /sys/class/power_supply/<Name>/status
+	Technology               string // /sys/class/power_supply/<Name>/technology
+	Temp                     *int64 // /sys/class/power_supply/<Name>/temp
+	TempAlertMax             *int64 // /sys/class/power_supply/<Name>/temp_alert_max
+	TempAlertMin             *int64 // /sys/class/power_supply/<Name>/temp_alert_min
+	TempAmbient              *int64 // /sys/class/power_supply/<Name>/temp_ambient
+	TempAmbientMax           *int64 // /sys/class/power_supply/<Name>/temp_ambient_max
+	TempAmbientMin           *int64 // /sys/class/power_supply/<Name>/temp_ambient_min
+	TempMax                  *int64 // /sys/class/power_supply/<Name>/temp_max
+	TempMin                  *int64 // /sys/class/power_supply/<Name>/temp_min
+	TimeToEmptyAvg           *int64 // /sys/class/power_supply/<Name>/time_to_empty_avg
+	TimeToEmptyNow           *int64 // /sys/class/power_supply/<Name>/time_to_empty_now
+	TimeToFullAvg            *int64 // /sys/class/power_supply/<Name>/time_to_full_avg
+	TimeToFullNow            *int64 // /sys/class/power_supply/<Name>/time_to_full_now
+	Type                     string // /sys/class/power_supply/<Name>/type
+	UsbType                  string // /sys/class/power_supply/<Name>/usb_type
+	VoltageAvg               *int64 // /sys/class/power_supply/<Name>/voltage_avg
+	VoltageBoot              *int64 // /sys/class/power_supply/<Name>/voltage_boot
+	VoltageMax               *int64 // /sys/class/power_supply/<Name>/voltage_max
+	VoltageMaxDesign         *int64 // /sys/class/power_supply/<Name>/voltage_max_design
+	VoltageMin               *int64 // /sys/class/power_supply/<Name>/voltage_min
+	VoltageMinDesign         *int64 // /sys/class/power_supply/<Name>/voltage_min_design
+	VoltageNow               *int64 // /sys/class/power_supply/<Name>/voltage_now
+	VoltageOCV               *int64 // /sys/class/power_supply/<Name>/voltage_ocv
 }
 
-// PowerSupplyClass is a collection of every power supply in /sys/class/power_supply/.
+// PowerSupplyClass is a collection of every power supply in
+// /sys/class/power_supply.
+//
 // The map keys are the names of the power supplies.
 type PowerSupplyClass map[string]PowerSupply
 
-// PowerSupplyClass returns info for all power supplies read from /sys/class/power_supply/.
+// PowerSupplyClass returns info for all power supplies read from
+// /sys/class/power_supply.
 func (fs FS) PowerSupplyClass() (PowerSupplyClass, error) {
 	path := fs.sys.Path("class/power_supply")
 
-	powerSupplyDirs, err := ioutil.ReadDir(path)
+	dirs, err := ioutil.ReadDir(path)
 	if err != nil {
-		return PowerSupplyClass{}, fmt.Errorf("cannot access %s dir %s", path, err)
+		return nil, fmt.Errorf("failed to list power supplies at %q: %v", path, err)
 	}
 
-	powerSupplyClass := PowerSupplyClass{}
-	for _, powerSupplyDir := range powerSupplyDirs {
-		powerSupply, err := powerSupplyClass.parsePowerSupply(path + "/" + powerSupplyDir.Name())
+	psc := make(PowerSupplyClass, len(dirs))
+	for _, d := range dirs {
+		ps, err := parsePowerSupply(filepath.Join(path, d.Name()))
 		if err != nil {
 			return nil, err
 		}
-		powerSupply.Name = powerSupplyDir.Name()
-		powerSupplyClass[powerSupplyDir.Name()] = *powerSupply
+
+		ps.Name = d.Name()
+		psc[d.Name()] = *ps
 	}
-	return powerSupplyClass, nil
+
+	return psc, nil
 }
 
-func (psc PowerSupplyClass) parsePowerSupply(powerSupplyPath string) (*PowerSupply, error) {
-	powerSupply := PowerSupply{}
-	powerSupplyElem := reflect.ValueOf(&powerSupply).Elem()
-	powerSupplyType := reflect.TypeOf(powerSupply)
+func parsePowerSupply(path string) (*PowerSupply, error) {
+	files, err := ioutil.ReadDir(path)
+	if err != nil {
+		return nil, err
+	}
 
-	//start from 1 - skip the Name field
-	for i := 1; i < powerSupplyElem.NumField(); i++ {
-		fieldType := powerSupplyType.Field(i)
-		fieldValue := powerSupplyElem.Field(i)
+	var ps PowerSupply
+	for _, f := range files {
+		if f.IsDir() {
+			continue
+		}
 
-		if fieldType.Tag.Get("fileName") == "" {
-			panic(fmt.Errorf("field %s does not have a filename tag", fieldType.Name))
+		name := filepath.Join(path, f.Name())
+		value, err := util.SysReadFile(name)
+		if err != nil {
+			return nil, fmt.Errorf("failed to read file %q: %v", name, err)
 		}
 
-		value, err := util.SysReadFile(powerSupplyPath + "/" + fieldType.Tag.Get("fileName"))
+		vp := util.NewValueParser(value)
 
-		if err != nil {
-			if os.IsNotExist(err) || err.Error() == "operation not supported" || err.Error() == "invalid argument" {
-				continue
-			}
-			return nil, fmt.Errorf("could not access file %s: %s", fieldType.Tag.Get("fileName"), err)
+		switch f.Name() {
+		case "authentic":
+			ps.Authentic = vp.PInt64()
+		case "calibrate":
+			ps.Calibrate = vp.PInt64()
+		case "capacity":
+			ps.Capacity = vp.PInt64()
+		case "capacity_alert_max":
+			ps.CapacityAlertMax = vp.PInt64()
+		case "capacity_alert_min":
+			ps.CapacityAlertMin = vp.PInt64()
+		case "capacity_level":
+			ps.CapacityLevel = value
+		case "charge_avg":
+			ps.ChargeAvg = vp.PInt64()
+		case "charge_control_limit":
+			ps.ChargeControlLimit = vp.PInt64()
+		case "charge_control_limit_max":
+			ps.ChargeControlLimitMax = vp.PInt64()
+		case "charge_counter":
+			ps.ChargeCounter = vp.PInt64()
+		case "charge_empty":
+			ps.ChargeEmpty = vp.PInt64()
+		case "charge_empty_design":
+			ps.ChargeEmptyDesign = vp.PInt64()
+		case "charge_full":
+			ps.ChargeFull = vp.PInt64()
+		case "charge_full_design":
+			ps.ChargeFullDesign = vp.PInt64()
+		case "charge_now":
+			ps.ChargeNow = vp.PInt64()
+		case "charge_term_current":
+			ps.ChargeTermCurrent = vp.PInt64()
+		case "charge_type":
+			ps.ChargeType = value
+		case "constant_charge_current":
+			ps.ConstantChargeCurrent = vp.PInt64()
+		case "constant_charge_current_max":
+			ps.ConstantChargeCurrentMax = vp.PInt64()
+		case "constant_charge_voltage":
+			ps.ConstantChargeVoltage = vp.PInt64()
+		case "constant_charge_voltage_max":
+			ps.ConstantChargeVoltageMax = vp.PInt64()
+		case "current_avg":
+			ps.CurrentAvg = vp.PInt64()
+		case "current_boot":
+			ps.CurrentBoot = vp.PInt64()
+		case "current_max":
+			ps.CurrentMax = vp.PInt64()
+		case "current_now":
+			ps.CurrentNow = vp.PInt64()
+		case "cycle_count":
+			ps.CycleCount = vp.PInt64()
+		case "energy_avg":
+			ps.EnergyAvg = vp.PInt64()
+		case "energy_empty":
+			ps.EnergyEmpty = vp.PInt64()
+		case "energy_empty_design":
+			ps.EnergyEmptyDesign = vp.PInt64()
+		case "energy_full":
+			ps.EnergyFull = vp.PInt64()
+		case "energy_full_design":
+			ps.EnergyFullDesign = vp.PInt64()
+		case "energy_now":
+			ps.EnergyNow = vp.PInt64()
+		case "health":
+			ps.Health = value
+		case "input_current_limit":
+			ps.InputCurrentLimit = vp.PInt64()
+		case "manufacturer":
+			ps.Manufacturer = value
+		case "model_name":
+			ps.ModelName = value
+		case "online":
+			ps.Online = vp.PInt64()
+		case "power_avg":
+			ps.PowerAvg = vp.PInt64()
+		case "power_now":
+			ps.PowerNow = vp.PInt64()
+		case "precharge_current":
+			ps.PrechargeCurrent = vp.PInt64()
+		case "present":
+			ps.Present = vp.PInt64()
+		case "scope":
+			ps.Scope = value
+		case "serial_number":
+			ps.SerialNumber = value
+		case "status":
+			ps.Status = value
+		case "technology":
+			ps.Technology = value
+		case "temp":
+			ps.Temp = vp.PInt64()
+		case "temp_alert_max":
+			ps.TempAlertMax = vp.PInt64()
+		case "temp_alert_min":
+			ps.TempAlertMin = vp.PInt64()
+		case "temp_ambient":
+			ps.TempAmbient = vp.PInt64()
+		case "temp_ambient_max":
+			ps.TempAmbientMax = vp.PInt64()
+		case "temp_ambient_min":
+			ps.TempAmbientMin = vp.PInt64()
+		case "temp_max":
+			ps.TempMax = vp.PInt64()
+		case "temp_min":
+			ps.TempMin = vp.PInt64()
+		case "time_to_empty_avg":
+			ps.TimeToEmptyAvg = vp.PInt64()
+		case "time_to_empty_now":
+			ps.TimeToEmptyNow = vp.PInt64()
+		case "time_to_full_avg":
+			ps.TimeToFullAvg = vp.PInt64()
+		case "time_to_full_now":
+			ps.TimeToFullNow = vp.PInt64()
+		case "type":
+			ps.Type = value
+		case "usb_type":
+			ps.UsbType = value
+		case "voltage_avg":
+			ps.VoltageAvg = vp.PInt64()
+		case "voltage_boot":
+			ps.VoltageBoot = vp.PInt64()
+		case "voltage_max":
+			ps.VoltageMax = vp.PInt64()
+		case "voltage_max_design":
+			ps.VoltageMaxDesign = vp.PInt64()
+		case "voltage_min":
+			ps.VoltageMin = vp.PInt64()
+		case "voltage_min_design":
+			ps.VoltageMinDesign = vp.PInt64()
+		case "voltage_now":
+			ps.VoltageNow = vp.PInt64()
+		case "voltage_ocv":
+			ps.VoltageOCV = vp.PInt64()
 		}
 
-		switch fieldValue.Kind() {
-		case reflect.String:
-			fieldValue.SetString(value)
-		case reflect.Ptr:
-			var int64ptr *int64
-			switch fieldValue.Type() {
-			case reflect.TypeOf(int64ptr):
-				var intValue int64
-				if strings.HasPrefix(value, "0x") {
-					intValue, err = strconv.ParseInt(value[2:], 16, 64)
-					if err != nil {
-						return nil, fmt.Errorf("expected hex value for %s, got: %s", fieldType.Name, value)
-					}
-				} else {
-					intValue, err = strconv.ParseInt(value, 10, 64)
-					if err != nil {
-						return nil, fmt.Errorf("expected Uint64 value for %s, got: %s", fieldType.Name, value)
-					}
-				}
-				fieldValue.Set(reflect.ValueOf(&intValue))
-			default:
-				return nil, fmt.Errorf("unhandled pointer type %q", fieldValue.Type())
-			}
-		default:
-			return nil, fmt.Errorf("unhandled type %q", fieldValue.Kind())
+		if err := vp.Err(); err != nil {
+			return nil, err
 		}
 	}
 
-	return &powerSupply, nil
+	return &ps, nil
 }


Test patch:
diff --git a/internal/util/valueparser_test.go b/internal/util/valueparser_test.go
new file mode 100644
index 0000000..ad94bb2
--- /dev/null
+++ b/internal/util/valueparser_test.go
@@ -0,0 +1,92 @@
+// Copyright 2019 The Prometheus Authors
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package util_test
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/prometheus/procfs/internal/util"
+)
+
+func TestValueParser(t *testing.T) {
+	tests := []struct {
+		name string
+		v    string
+		ok   bool
+		fn   func(t *testing.T, vp *util.ValueParser)
+	}{
+		{
+			name: "bad PInt64",
+			v:    "hello",
+			fn: func(_ *testing.T, vp *util.ValueParser) {
+				_ = vp.PInt64()
+			},
+		},
+		{
+			name: "bad hex PInt64",
+			v:    "0xhello",
+			fn: func(_ *testing.T, vp *util.ValueParser) {
+				_ = vp.PInt64()
+			},
+		},
+		{
+			name: "ok PInt64",
+			v:    "1",
+			ok:   true,
+			fn: func(t *testing.T, vp *util.ValueParser) {
+				want := int64(1)
+				got := vp.PInt64()
+
+				if diff := cmp.Diff(&want, got); diff != "" {
+					t.Fatalf("unexpected integer (-want +got):\n%s", diff)
+				}
+			},
+		},
+		{
+			name: "ok hex PInt64",
+			v:    "0xff",
+			ok:   true,
+			fn: func(t *testing.T, vp *util.ValueParser) {
+				want := int64(255)
+				got := vp.PInt64()
+
+				if diff := cmp.Diff(&want, got); diff != "" {
+					t.Fatalf("unexpected integer (-want +got):\n%s", diff)
+				}
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			vp := util.NewValueParser(tt.v)
+			tt.fn(t, vp)
+
+			err := vp.Err()
+			if err != nil {
+				if tt.ok {
+					t.Fatalf("unexpected error: %v", err)
+				}
+
+				t.Logf("OK err: %v", err)
+				return
+			}
+
+			if !tt.ok {
+				t.Fatal("expected an error, but none occurred")
+			}
+		})
+	}
+}
diff --git a/sysfs/class_power_supply_test.go b/sysfs/class_power_supply_test.go
index d765492..cd66fc4 100644
--- a/sysfs/class_power_supply_test.go
+++ b/sysfs/class_power_supply_test.go
@@ -16,26 +16,26 @@
 package sysfs
 
 import (
-	"encoding/json"
-	"reflect"
 	"testing"
+
+	"github.com/google/go-cmp/cmp"
 )
 
 func TestPowerSupplyClass(t *testing.T) {
 	fs, err := NewFS(sysTestFixtures)
 	if err != nil {
-		t.Fatal(err)
+		t.Fatalf("failed to open filesystem: %v", err)
 	}
 
-	psc, err := fs.PowerSupplyClass()
+	got, err := fs.PowerSupplyClass()
 	if err != nil {
-		t.Fatal(err)
+		t.Fatalf("failed to parse power supply class: %v", err)
 	}
 
 	var (
-		acOnline             int64 = 0
+		acOnline             int64
 		bat0Capacity         int64 = 98
-		bat0CycleCount       int64 = 0
+		bat0CycleCount       int64
 		bat0EnergyFull       int64 = 50060000
 		bat0EnergyFullDesign int64 = 47520000
 		bat0EnergyNow        int64 = 49450000
@@ -45,7 +45,7 @@ func TestPowerSupplyClass(t *testing.T) {
 		bat0VoltageNow       int64 = 12229000
 	)
 
-	powerSupplyClass := PowerSupplyClass{
+	want := PowerSupplyClass{
 		"AC": {
 			Name:   "AC",
 			Type:   "Mains",
@@ -72,9 +72,7 @@ func TestPowerSupplyClass(t *testing.T) {
 		},
 	}
 
-	if !reflect.DeepEqual(powerSupplyClass, psc) {
-		want, _ := json.Marshal(powerSupplyClass)
-		get, _ := json.Marshal(psc)
-		t.Errorf("Result not correct: want %v, have %v.", string(want), string(get))
+	if diff := cmp.Diff(want, got); diff != "" {
+		t.Fatalf("unexpected power supply class (-want +got):\n%s", diff)
 	}
 }