Hi Alistair,
On Thu, 8 Jan 2009 19:53:04 +0000, Alistair John Strachan wrote:
> Find attached an alternative method which solves (I believe only potential)
> problems Jean pointed out at the time of the last thread. I think it's safe.
>
> I've compromised by finding the first open bracket (it's fine if there are no
> brackets) and trimming any space from the sub-string. For example, here are
> the existing transformations:
>
> "AW9D-MAX (Intel i975-ICH7)" -> "AW9D-MAX"
> "AT8 32X(ATI RD580-ULI M1575)" -> "AT8 32X"
> "IP35 Pro(Intel P35-ICH9R)" -> "IP35 Pro"
> "IN9 32X MAX(680i-MCP55PXE)" -> "IN9 32X MAX" (adding RSN)
>
> And here are some hypothetical ones:
>
> "AW9D (Blah)" -> "AW9D"
> "Abit Magic" -> "Abit Magic"
>
> In the AW9D case, "AW9D" and "AW9D-MAX" are not considered to be the same
> motherboard, because the sub-string's length must exactly match that of the
> string in the motherboard entry. This is different to regular "strncasecmp"
> where they would be considered the same.
>
> Find the patch attached. I'll probably go for this version if nobody finds any
> issues with it, otherwise the original patch I sent out is sufficient (since
> at the moment there are no boards falling into this hypothetical trap).
Review:
Please add a patch summary and your Signed-off-by line so that I can
push the patch upstream.
> diff --git a/drivers/hwmon/abituguru3.c b/drivers/hwmon/abituguru3.c
> index 70bb854..16d3142 100644
> --- a/drivers/hwmon/abituguru3.c
> +++ b/drivers/hwmon/abituguru3.c
> @@ -279,7 +279,7 @@ static const struct abituguru3_motherboard_info abituguru3_motherboards[] = {
> { "OTES1 Fan", 36, 2, 60, 1, 0 },
> { NULL, 0, 0, 0, 0, 0 } }
> },
> - { 0x0011, "AT8 32X(ATI RD580-ULI M1575)", {
> + { 0x0011, "AT8 32X", {
> { "CPU Core", 0, 0, 10, 1, 0 },
> { "DDR", 1, 0, 20, 1, 0 },
> { "DDR VTT", 2, 0, 10, 1, 0 },
> @@ -402,7 +402,7 @@ static const struct abituguru3_motherboard_info abituguru3_motherboards[] = {
> { "AUX3 Fan", 36, 2, 60, 1, 0 },
> { NULL, 0, 0, 0, 0, 0 } }
> },
> - { 0x0016, "AW9D-MAX (Intel i975-ICH7)", {
> + { 0x0016, "AW9D-MAX", {
> { "CPU Core", 0, 0, 10, 1, 0 },
> { "DDR2", 1, 0, 20, 1, 0 },
> { "DDR2 VTT", 2, 0, 10, 1, 0 },
> @@ -509,7 +509,7 @@ static const struct abituguru3_motherboard_info abituguru3_motherboards[] = {
> { "AUX3 FAN", 36, 2, 60, 1, 0 },
> { NULL, 0, 0, 0, 0, 0 } }
> },
> - { 0x001A, "IP35 Pro(Intel P35-ICH9R)", {
> + { 0x001A, "IP35 Pro", {
> { "CPU Core", 0, 0, 10, 1, 0 },
> { "DDR2", 1, 0, 20, 1, 0 },
> { "DDR2 VTT", 2, 0, 10, 1, 0 },
> @@ -1128,6 +1128,7 @@ static int __init abituguru3_dmi_detect(void)
> {
> const char *board_vendor, *board_name;
> int i, err = (force) ? 1 : -ENODEV;
> + size_t sublen;
>
> board_vendor = dmi_get_system_info(DMI_BOARD_VENDOR);
> if (!board_vendor || strcmp(board_vendor, "http://www.abit.com.tw/";))
> @@ -1137,9 +1138,20 @@ static int __init abituguru3_dmi_detect(void)
> if (!board_name)
> return err;
>
> + /* At the moment, we don't care about the part of the vendor
> + * DMI string contained in brackets. Truncate the string at
> + * the first occurance of a bracket. Trim any trailing space
Spelling: occurrence.
> + * from the substring.
> + */
> + sublen = strcspn(board_name, "(");
I didn't even know this function existed ;)
> + while(sublen > 0 && board_name[sublen - 1] == ' ')
Coding style: space between "while" and opening parenthesis.
> + sublen--;
> +
> for (i = 0; abituguru3_motherboards[i].id; i++) {
> const char *dmi_name = abituguru3_motherboards[i].dmi_name;
> - if (dmi_name && !strcmp(dmi_name, board_name))
> + if (!dmi_name || strlen(dmi_name) != sublen)
> + continue;
> + if (!strncasecmp(board_name, dmi_name, sublen))
> break;
> }
>
Other than that, the patch looks OK to me.
Thanks,
--
Jean Delvare
