Date: Tue, 30 Mar 2004 11:43:26 -0800 (PST) From: Nate Lawson <nate@root.org> To: Takanori Watanabe <takawata@FreeBSD.org> Cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/dev/acpica acpi.c Message-ID: <20040330113454.I81785@root.org> In-Reply-To: <20040327162613.6C32A16A4E1@hub.freebsd.org> References: <20040327162613.6C32A16A4E1@hub.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040330113454.I81785>