cvs commit: src/sys/dev/acpica acpi.c

[Nate Lawson]

On Sat, 27 Mar 2004, Takanori Watanabe wrote:
>   Modified files:
>     sys/dev/acpica       acpi.c
>   Log:
>   Add ACPI PnP string. This affects devinfo(8) output with -v option.
>
>   Revision  Changes    Path
>   1.129     +46 -0     src/sys/dev/acpica/acpi.c

Excellent, thanks for this work.

> --- src/sys/dev/acpica/acpi.c:1.128	Thu Mar 18 23:05:01 2004
> +++ src/sys/dev/acpica/acpi.c	Sat Mar 27 08:26:00 2004
> @@ -140,6 +140,50 @@
>  static int	acpi_supported_sleep_state_sysctl(SYSCTL_HANDLER_ARGS);
>  static int	acpi_sleep_state_sysctl(SYSCTL_HANDLER_ARGS);
>  static int	acpi_pm_func(u_long cmd, void *arg, ...);
> +static int	acpi_child_location_str_method(device_t acdev, device_t child,
> +					       char *buf, size_t buflen);
> +static int	acpi_child_pnpinfo_str_method(device_t acdev, device_t child,
> +					      char *buf, size_t buflen);
> +
> +int
> +acpi_child_location_str_method(device_t cbdev, device_t child, char *buf,
> +    size_t buflen)
> +{
> +	struct acpi_device *dinfo= device_get_ivars(child);

style -- space before '='.  You should remove this initialization anyway
since it is duplicated below.

> +
> +	dinfo = device_get_ivars(child);
> +	if(dinfo->ad_handle)

style -- space after "if"

> +		snprintf(buf, buflen, "path=%s", acpi_name(dinfo->ad_handle));
> +	else
> +		snprintf(buf, buflen, "magic=unknown");
> +	return (0);
> +}

style -- use 4 spaces instead of a tab, tabs for 8-space indents.  This
applies to the whole commit.  See other parts of this file.

> +int
> +acpi_child_pnpinfo_str_method(device_t cbdev, device_t child, char *buf,
> +    size_t buflen)
> +{
> +	struct acpi_device *dinfo = device_get_ivars(child);

Duplicate initialization.

> +	ACPI_DEVICE_INFO adinfo;
> +	ACPI_BUFFER adbuf = {sizeof(adinfo), &adinfo};

Please dynamically allocate buffer elements as done elsewhere.  Our kernel
stack is restricted.

> +	char * end;

style -- space after *.

> +	int error;
> +
> +	dinfo = device_get_ivars(child);
> +	error = AcpiGetObjectInfo(dinfo->ad_handle, &adbuf);
> +
> +	if(error)

style -- space needed after "if"

> +		snprintf(buf, buflen, "Unknown");
> +	else
> +		snprintf(buf, buflen, "_HID=%s _UID=%u",
> +			 (adinfo.Valid & ACPI_VALID_HID)?
> +			 adinfo.HardwareId.Value : "UNKNOWN",
> +			 (unsigned int)((adinfo.Valid & ACPI_VALID_UID)?
> +			strtoul(adinfo.UniqueId.Value, &end, 10):0 ));
> +
> +	return (0);
> +}

How about adding first _CID value if it's valid?

Instead of cast, just use "%ul" as the format.  This would simplify the
statement to:

>              (adinfo.Valid & ACPI_VALID_UID) ?
>              strtoul(adinfo.UniqueId.Value, &end, 10) : 0);

Also note spaces added above.

Thanks,
Nate