* [PATCH 2/8] ACPI, x86: fix Dell M6600 ACPI reboot regression via DMI
2012-06-30 5:07 ` [PATCH 1/8] ACPI sysfs.c strlen fix Len Brown
@ 2012-06-30 5:07 ` Len Brown
2012-07-06 11:09 ` Ingo Molnar
2012-06-30 5:07 ` [PATCH 3/8] ACPI: Make acpi_skip_timer_override cover all source_irq==0 cases Len Brown
` (6 subsequent siblings)
7 siblings, 1 reply; 29+ messages in thread
From: Len Brown @ 2012-06-30 5:07 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: Len Brown, x86, linux-kernel
From: Zhang Rui <rui.zhang@intel.com>
Dell Precision M6600 is known to require PCI reboot, so add it to
the reboot blacklist in pci_reboot_dmi_table[].
https://bugzilla.kernel.org/show_bug.cgi?id=42749
cc: x86@kernel.org
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
arch/x86/kernel/reboot.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 79c45af..412db57 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -451,6 +451,14 @@ static struct dmi_system_id __initdata reboot_dmi_table[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "OptiPlex 990"),
},
},
+ { /* Handle problems with rebooting on the Precision M6600. */
+ .callback = set_pci_reboot,
+ .ident = "Dell OptiPlex 990",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Precision M6600"),
+ },
+ },
{ }
};
--
1.7.11.1.104.ge7b44f1
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 2/8] ACPI, x86: fix Dell M6600 ACPI reboot regression via DMI
2012-06-30 5:07 ` [PATCH 2/8] ACPI, x86: fix Dell M6600 ACPI reboot regression via DMI Len Brown
@ 2012-07-06 11:09 ` Ingo Molnar
0 siblings, 0 replies; 29+ messages in thread
From: Ingo Molnar @ 2012-07-06 11:09 UTC (permalink / raw)
To: Len Brown; +Cc: linux-acpi, linux-pm, linux-kernel, Zhang Rui, x86, Len Brown
* Len Brown <lenb@kernel.org> wrote:
> From: Zhang Rui <rui.zhang@intel.com>
>
> Dell Precision M6600 is known to require PCI reboot, so add it to
> the reboot blacklist in pci_reboot_dmi_table[].
>
> https://bugzilla.kernel.org/show_bug.cgi?id=42749
>
> cc: x86@kernel.org
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> Signed-off-by: Len Brown <len.brown@intel.com>
> ---
> arch/x86/kernel/reboot.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
Acked-by: Ingo Molnar <mingo@kernel.org>
Thanks,
Ingo
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 3/8] ACPI: Make acpi_skip_timer_override cover all source_irq==0 cases
2012-06-30 5:07 ` [PATCH 1/8] ACPI sysfs.c strlen fix Len Brown
2012-06-30 5:07 ` [PATCH 2/8] ACPI, x86: fix Dell M6600 ACPI reboot regression via DMI Len Brown
@ 2012-06-30 5:07 ` Len Brown
2012-06-30 5:07 ` [PATCH 4/8] ACPI: Remove one board specific WARN when ignoring timer overriding Len Brown
` (5 subsequent siblings)
7 siblings, 0 replies; 29+ messages in thread
From: Len Brown @ 2012-06-30 5:07 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: Len Brown, linux-kernel
From: Feng Tang <feng.tang@intel.com>
Currently when acpi_skip_timer_override is set, it only cover the
(source_irq == 0 && global_irq == 2) cases. While there is also
platform which need use this option and its global_irq is not 2.
This patch will extend acpi_skip_timer_override to cover all
timer overriding cases as long as the source irq is 0.
This is the first part of a fix to kernel bug bugzilla 40002:
"IRQ 0 assigned to VGA"
https://bugzilla.kernel.org/show_bug.cgi?id=40002
Reported-and-tested-by: Szymon Kowalczyk <fazerxlo@o2.pl>
Signed-off-by: Feng Tang <feng.tang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
arch/x86/kernel/acpi/boot.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 8afb693..e7c698e 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -422,12 +422,14 @@ acpi_parse_int_src_ovr(struct acpi_subtable_header * header,
return 0;
}
- if (intsrc->source_irq == 0 && intsrc->global_irq == 2) {
+ if (intsrc->source_irq == 0) {
if (acpi_skip_timer_override) {
- printk(PREFIX "BIOS IRQ0 pin2 override ignored.\n");
+ printk(PREFIX "BIOS IRQ0 override ignored.\n");
return 0;
}
- if (acpi_fix_pin2_polarity && (intsrc->inti_flags & ACPI_MADT_POLARITY_MASK)) {
+
+ if ((intsrc->global_irq == 2) && acpi_fix_pin2_polarity
+ && (intsrc->inti_flags & ACPI_MADT_POLARITY_MASK)) {
intsrc->inti_flags &= ~ACPI_MADT_POLARITY_MASK;
printk(PREFIX "BIOS IRQ0 pin2 override: forcing polarity to high active.\n");
}
@@ -1334,7 +1336,7 @@ static int __init dmi_disable_acpi(const struct dmi_system_id *d)
}
/*
- * Force ignoring BIOS IRQ0 pin2 override
+ * Force ignoring BIOS IRQ0 override
*/
static int __init dmi_ignore_irq0_timer_override(const struct dmi_system_id *d)
{
@@ -1344,7 +1346,7 @@ static int __init dmi_ignore_irq0_timer_override(const struct dmi_system_id *d)
*/
if (!acpi_skip_timer_override) {
WARN(1, KERN_ERR "ati_ixp4x0 quirk not complete.\n");
- pr_notice("%s detected: Ignoring BIOS IRQ0 pin2 override\n",
+ pr_notice("%s detected: Ignoring BIOS IRQ0 override\n",
d->ident);
acpi_skip_timer_override = 1;
}
@@ -1438,7 +1440,7 @@ static struct dmi_system_id __initdata acpi_dmi_table_late[] = {
* is enabled. This input is incorrectly designated the
* ISA IRQ 0 via an interrupt source override even though
* it is wired to the output of the master 8259A and INTIN0
- * is not connected at all. Force ignoring BIOS IRQ0 pin2
+ * is not connected at all. Force ignoring BIOS IRQ0
* override in that cases.
*/
{
--
1.7.11.1.104.ge7b44f1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 4/8] ACPI: Remove one board specific WARN when ignoring timer overriding
2012-06-30 5:07 ` [PATCH 1/8] ACPI sysfs.c strlen fix Len Brown
2012-06-30 5:07 ` [PATCH 2/8] ACPI, x86: fix Dell M6600 ACPI reboot regression via DMI Len Brown
2012-06-30 5:07 ` [PATCH 3/8] ACPI: Make acpi_skip_timer_override cover all source_irq==0 cases Len Brown
@ 2012-06-30 5:07 ` Len Brown
2012-06-30 5:07 ` [PATCH 5/8] ACPI: Add a quirk for "AMILO PRO V2030" to ignore the " Len Brown
` (4 subsequent siblings)
7 siblings, 0 replies; 29+ messages in thread
From: Len Brown @ 2012-06-30 5:07 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: Len Brown, linux-kernel
From: Feng Tang <feng.tang@intel.com>
Current WARN msg is only for the ati_ixp4x0 board, while this function
is used by mulitple platforms. So this one board specific warning
is not appropriate any more.
Signed-off-by: Feng Tang <feng.tang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
arch/x86/kernel/acpi/boot.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index e7c698e..3a6afba 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -1340,12 +1340,7 @@ static int __init dmi_disable_acpi(const struct dmi_system_id *d)
*/
static int __init dmi_ignore_irq0_timer_override(const struct dmi_system_id *d)
{
- /*
- * The ati_ixp4x0_rev() early PCI quirk should have set
- * the acpi_skip_timer_override flag already:
- */
if (!acpi_skip_timer_override) {
- WARN(1, KERN_ERR "ati_ixp4x0 quirk not complete.\n");
pr_notice("%s detected: Ignoring BIOS IRQ0 override\n",
d->ident);
acpi_skip_timer_override = 1;
--
1.7.11.1.104.ge7b44f1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 5/8] ACPI: Add a quirk for "AMILO PRO V2030" to ignore the timer overriding
2012-06-30 5:07 ` [PATCH 1/8] ACPI sysfs.c strlen fix Len Brown
` (2 preceding siblings ...)
2012-06-30 5:07 ` [PATCH 4/8] ACPI: Remove one board specific WARN when ignoring timer overriding Len Brown
@ 2012-06-30 5:07 ` Len Brown
2012-06-30 5:07 ` [PATCH 6/8] ACPI, APEI, Avoid too much error reporting in runtime Len Brown
` (3 subsequent siblings)
7 siblings, 0 replies; 29+ messages in thread
From: Len Brown @ 2012-06-30 5:07 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: Len Brown, linux-kernel
From: Feng Tang <feng.tang@intel.com>
This is the 2nd part of fix for kernel bugzilla 40002:
"IRQ 0 assigned to VGA"
https://bugzilla.kernel.org/show_bug.cgi?id=40002
The root cause is the buggy FW, whose ACPI tables assign the GSI 16
to 2 irqs 0 and 16(VGA), and the VGA is the right owner of GSI 16.
So add a quirk to ignore the irq0 overriding GSI 16 for the
FUJITSU SIEMENS AMILO PRO V2030 platform will solve this issue.
Reported-and-tested-by: Szymon Kowalczyk <fazerxlo@o2.pl>
Signed-off-by: Feng Tang <feng.tang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
arch/x86/kernel/acpi/boot.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 3a6afba..b2297e5 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -1470,6 +1470,14 @@ static struct dmi_system_id __initdata acpi_dmi_table_late[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "HP Compaq 6715b"),
},
},
+ {
+ .callback = dmi_ignore_irq0_timer_override,
+ .ident = "FUJITSU SIEMENS",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU SIEMENS"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "AMILO PRO V2030"),
+ },
+ },
{}
};
--
1.7.11.1.104.ge7b44f1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 6/8] ACPI, APEI, Avoid too much error reporting in runtime
2012-06-30 5:07 ` [PATCH 1/8] ACPI sysfs.c strlen fix Len Brown
` (3 preceding siblings ...)
2012-06-30 5:07 ` [PATCH 5/8] ACPI: Add a quirk for "AMILO PRO V2030" to ignore the " Len Brown
@ 2012-06-30 5:07 ` Len Brown
2012-06-30 5:07 ` [PATCH 7/8] ACPI video: Still use ACPI backlight control if _DOS doesn't exist Len Brown
` (2 subsequent siblings)
7 siblings, 0 replies; 29+ messages in thread
From: Len Brown @ 2012-06-30 5:07 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: Len Brown, linux-kernel, stable, Huang Ying
From: Huang Ying <ying.huang@intel.com>
This patch fixed the following bug.
https://bugzilla.kernel.org/show_bug.cgi?id=43282
This is caused by a firmware bug checking (checking generic address
register provided by firmware) in runtime. The checking should be
done in address mapping time instead of runtime to avoid too much
error reporting in runtime.
Reported-by: Pawel Sikora <pluto@agmk.net>
Signed-off-by: Huang Ying <ying.huang@intel.com>
Tested-by: Jean Delvare <khali@linux-fr.org>
Cc: stable@vger.kernel.org
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/apei/apei-base.c | 17 +++++++++++++++--
drivers/acpi/apei/apei-internal.h | 9 +++++++++
drivers/acpi/apei/ghes.c | 6 +++---
3 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
index 5577762..6686b1e 100644
--- a/drivers/acpi/apei/apei-base.c
+++ b/drivers/acpi/apei/apei-base.c
@@ -243,7 +243,7 @@ static int pre_map_gar_callback(struct apei_exec_context *ctx,
u8 ins = entry->instruction;
if (ctx->ins_table[ins].flags & APEI_EXEC_INS_ACCESS_REGISTER)
- return acpi_os_map_generic_address(&entry->register_region);
+ return apei_map_generic_address(&entry->register_region);
return 0;
}
@@ -276,7 +276,7 @@ static int post_unmap_gar_callback(struct apei_exec_context *ctx,
u8 ins = entry->instruction;
if (ctx->ins_table[ins].flags & APEI_EXEC_INS_ACCESS_REGISTER)
- acpi_os_unmap_generic_address(&entry->register_region);
+ apei_unmap_generic_address(&entry->register_region);
return 0;
}
@@ -606,6 +606,19 @@ static int apei_check_gar(struct acpi_generic_address *reg, u64 *paddr,
return 0;
}
+int apei_map_generic_address(struct acpi_generic_address *reg)
+{
+ int rc;
+ u32 access_bit_width;
+ u64 address;
+
+ rc = apei_check_gar(reg, &address, &access_bit_width);
+ if (rc)
+ return rc;
+ return acpi_os_map_generic_address(reg);
+}
+EXPORT_SYMBOL_GPL(apei_map_generic_address);
+
/* read GAR in interrupt (including NMI) or process context */
int apei_read(u64 *val, struct acpi_generic_address *reg)
{
diff --git a/drivers/acpi/apei/apei-internal.h b/drivers/acpi/apei/apei-internal.h
index cca240a..f220d64 100644
--- a/drivers/acpi/apei/apei-internal.h
+++ b/drivers/acpi/apei/apei-internal.h
@@ -7,6 +7,8 @@
#define APEI_INTERNAL_H
#include <linux/cper.h>
+#include <linux/acpi.h>
+#include <linux/acpi_io.h>
struct apei_exec_context;
@@ -68,6 +70,13 @@ static inline int apei_exec_run_optional(struct apei_exec_context *ctx, u8 actio
/* IP has been set in instruction function */
#define APEI_EXEC_SET_IP 1
+int apei_map_generic_address(struct acpi_generic_address *reg);
+
+static inline void apei_unmap_generic_address(struct acpi_generic_address *reg)
+{
+ acpi_os_unmap_generic_address(reg);
+}
+
int apei_read(u64 *val, struct acpi_generic_address *reg);
int apei_write(u64 val, struct acpi_generic_address *reg);
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 9b3cac0..1599566 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -301,7 +301,7 @@ static struct ghes *ghes_new(struct acpi_hest_generic *generic)
if (!ghes)
return ERR_PTR(-ENOMEM);
ghes->generic = generic;
- rc = acpi_os_map_generic_address(&generic->error_status_address);
+ rc = apei_map_generic_address(&generic->error_status_address);
if (rc)
goto err_free;
error_block_length = generic->error_block_length;
@@ -321,7 +321,7 @@ static struct ghes *ghes_new(struct acpi_hest_generic *generic)
return ghes;
err_unmap:
- acpi_os_unmap_generic_address(&generic->error_status_address);
+ apei_unmap_generic_address(&generic->error_status_address);
err_free:
kfree(ghes);
return ERR_PTR(rc);
@@ -330,7 +330,7 @@ err_free:
static void ghes_fini(struct ghes *ghes)
{
kfree(ghes->estatus);
- acpi_os_unmap_generic_address(&ghes->generic->error_status_address);
+ apei_unmap_generic_address(&ghes->generic->error_status_address);
}
enum {
--
1.7.11.1.104.ge7b44f1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 7/8] ACPI video: Still use ACPI backlight control if _DOS doesn't exist
2012-06-30 5:07 ` [PATCH 1/8] ACPI sysfs.c strlen fix Len Brown
` (4 preceding siblings ...)
2012-06-30 5:07 ` [PATCH 6/8] ACPI, APEI, Avoid too much error reporting in runtime Len Brown
@ 2012-06-30 5:07 ` Len Brown
2012-06-30 5:07 ` [PATCH 8/8] acpi_pad: fix power_saving thread deadlock Len Brown
2012-06-30 14:25 ` [PATCH 1/8] ACPI sysfs.c strlen fix Alan Stern
7 siblings, 0 replies; 29+ messages in thread
From: Len Brown @ 2012-06-30 5:07 UTC (permalink / raw)
To: linux-acpi, linux-pm
Cc: linux-kernel, Zhang Rui, Igor Murzov, stable, Len Brown
From: Zhang Rui <rui.zhang@intel.com>
This fixes a regression in 3.4-rc1 caused by commit
ea9f8856bd6d4ed45885b06a338f7362cd6c60e5
(ACPI video: Harden video bus adding.)
Some platforms don't have _DOS control method, but the ACPI
backlight still works.
We should not invoke _DOS for these platforms.
https://bugzilla.kernel.org/show_bug.cgi?id=43168
Cc: Igor Murzov <intergalactic.anonymous@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/video.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 9577b6f..4134b30 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -558,6 +558,8 @@ acpi_video_bus_DOS(struct acpi_video_bus *video, int bios_flag, int lcd_flag)
union acpi_object arg0 = { ACPI_TYPE_INTEGER };
struct acpi_object_list args = { 1, &arg0 };
+ if (!video->cap._DOS)
+ return 0;
if (bios_flag < 0 || bios_flag > 3 || lcd_flag < 0 || lcd_flag > 1)
return -EINVAL;
--
1.7.11.1.104.ge7b44f1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 8/8] acpi_pad: fix power_saving thread deadlock
2012-06-30 5:07 ` [PATCH 1/8] ACPI sysfs.c strlen fix Len Brown
` (5 preceding siblings ...)
2012-06-30 5:07 ` [PATCH 7/8] ACPI video: Still use ACPI backlight control if _DOS doesn't exist Len Brown
@ 2012-06-30 5:07 ` Len Brown
2012-06-30 14:25 ` [PATCH 1/8] ACPI sysfs.c strlen fix Alan Stern
7 siblings, 0 replies; 29+ messages in thread
From: Len Brown @ 2012-06-30 5:07 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: Len Brown, linux-kernel, stable, Stuart Hayes
From: Stuart Hayes <Stuart_Hayes@Dell.com>
The acpi_pad driver can get stuck in destroy_power_saving_task()
waiting for kthread_stop() to stop a power_saving thread. The problem
is that the isolated_cpus_lock mutex is owned when
destroy_power_saving_task() calls kthread_stop(), which waits for a
power_saving thread to end, and the power_saving thread tries to
acquire the isolated_cpus_lock when it calls round_robin_cpu(). This
patch fixes the issue by making round_robin_cpu() use its own mutex.
https://bugzilla.kernel.org/show_bug.cgi?id=42981
Cc: stable@vger.kernel.org
Signed-off-by: Stuart Hayes <Stuart_Hayes@Dell.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/acpi_pad.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/acpi_pad.c b/drivers/acpi/acpi_pad.c
index a43fa1a..1502c502 100644
--- a/drivers/acpi/acpi_pad.c
+++ b/drivers/acpi/acpi_pad.c
@@ -36,6 +36,7 @@
#define ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME "Processor Aggregator"
#define ACPI_PROCESSOR_AGGREGATOR_NOTIFY 0x80
static DEFINE_MUTEX(isolated_cpus_lock);
+static DEFINE_MUTEX(round_robin_lock);
static unsigned long power_saving_mwait_eax;
@@ -107,7 +108,7 @@ static void round_robin_cpu(unsigned int tsk_index)
if (!alloc_cpumask_var(&tmp, GFP_KERNEL))
return;
- mutex_lock(&isolated_cpus_lock);
+ mutex_lock(&round_robin_lock);
cpumask_clear(tmp);
for_each_cpu(cpu, pad_busy_cpus)
cpumask_or(tmp, tmp, topology_thread_cpumask(cpu));
@@ -116,7 +117,7 @@ static void round_robin_cpu(unsigned int tsk_index)
if (cpumask_empty(tmp))
cpumask_andnot(tmp, cpu_online_mask, pad_busy_cpus);
if (cpumask_empty(tmp)) {
- mutex_unlock(&isolated_cpus_lock);
+ mutex_unlock(&round_robin_lock);
return;
}
for_each_cpu(cpu, tmp) {
@@ -131,7 +132,7 @@ static void round_robin_cpu(unsigned int tsk_index)
tsk_in_cpu[tsk_index] = preferred_cpu;
cpumask_set_cpu(preferred_cpu, pad_busy_cpus);
cpu_weight[preferred_cpu]++;
- mutex_unlock(&isolated_cpus_lock);
+ mutex_unlock(&round_robin_lock);
set_cpus_allowed_ptr(current, cpumask_of(preferred_cpu));
}
--
1.7.11.1.104.ge7b44f1
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 1/8] ACPI sysfs.c strlen fix
2012-06-30 5:07 ` [PATCH 1/8] ACPI sysfs.c strlen fix Len Brown
` (6 preceding siblings ...)
2012-06-30 5:07 ` [PATCH 8/8] acpi_pad: fix power_saving thread deadlock Len Brown
@ 2012-06-30 14:25 ` Alan Stern
2012-07-27 1:39 ` [PATCH] ACPI: replace strlen("string") with sizeof("string") -1 Len Brown
7 siblings, 1 reply; 29+ messages in thread
From: Alan Stern @ 2012-06-30 14:25 UTC (permalink / raw)
To: Len Brown; +Cc: linux-acpi, linux-pm, linux-kernel, Len Brown, Pavel Vasilyev
On Sat, 30 Jun 2012, Len Brown wrote:
> From: Pavel Vasilyev <pavel@pavlinux.ru>
>
> Current code is ignoring the last character of "enable" and "disable"
> in comparisons.
>
> https://bugzilla.kernel.org/show_bug.cgi?id=33732
>
> Signed-off-by: Len Brown <len.brown@intel.com>
> ---
> drivers/acpi/sysfs.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
> index 9f66181..240a244 100644
> --- a/drivers/acpi/sysfs.c
> +++ b/drivers/acpi/sysfs.c
> @@ -173,7 +173,7 @@ static int param_set_trace_state(const char *val, struct kernel_param *kp)
> {
> int result = 0;
>
> - if (!strncmp(val, "enable", strlen("enable") - 1)) {
> + if (!strncmp(val, "enable", strlen("enable"))) {
Ironically, this is the less efficient of two possible fixes.
It would have been better to leave the "- 1" in place and change strlen
to sizeof. That's probably what the original author had in mind.
Alan Stern
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
2012-06-30 14:25 ` [PATCH 1/8] ACPI sysfs.c strlen fix Alan Stern
@ 2012-07-27 1:39 ` Len Brown
2012-07-30 14:26 ` Ian Campbell
` (2 more replies)
0 siblings, 3 replies; 29+ messages in thread
From: Len Brown @ 2012-07-27 1:39 UTC (permalink / raw)
To: Alan Stern; +Cc: linux-acpi, linux-pm, linux-kernel, Len Brown, Pavel Vasilyev
...both give the number of chars in the string
without the '\0', as strncmp() wants,
but sizeof() is compile-time.
Reported-by: Alan Stern <stern@rowland.harvard.edu>
Cc: Pavel Vasilyev <pavel@pavlinux.ru>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/sysfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
index 240a244..7c3f98b 100644
--- a/drivers/acpi/sysfs.c
+++ b/drivers/acpi/sysfs.c
@@ -173,7 +173,7 @@ static int param_set_trace_state(const char *val,
struct kernel_param *kp)
{
int result = 0;
- if (!strncmp(val, "enable", strlen("enable"))) {
+ if (!strncmp(val, "enable", sizeof("enable") - 1)) {
result = acpi_debug_trace(trace_method_name, trace_debug_level,
trace_debug_layer, 0);
if (result)
@@ -181,7 +181,7 @@ static int param_set_trace_state(const char *val,
struct kernel_param *kp)
goto exit;
}
- if (!strncmp(val, "disable", strlen("disable"))) {
+ if (!strncmp(val, "disable", sizeof("disable") - 1)) {
int name = 0;
result = acpi_debug_trace((char *)&name, trace_debug_level,
trace_debug_layer, 0);
--
1.7.12.rc0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
2012-07-27 1:39 ` [PATCH] ACPI: replace strlen("string") with sizeof("string") -1 Len Brown
@ 2012-07-30 14:26 ` Ian Campbell
2012-07-30 14:41 ` richard -rw- weinberger
2012-08-06 10:21 ` [linux-pm] " Pavel Machek
2012-08-06 10:26 ` Andreas Schwab
2 siblings, 1 reply; 29+ messages in thread
From: Ian Campbell @ 2012-07-30 14:26 UTC (permalink / raw)
To: Len Brown
Cc: Alan Stern, linux-acpi, linux-pm, Pavel Vasilyev, linux-kernel,
Len Brown
On Thu, 2012-07-26 at 21:39 -0400, Len Brown wrote:
> ...both give the number of chars in the string
> without the '\0', as strncmp() wants,
> but sizeof() is compile-time.
I thought gcc optimised strlen("string literal") into a compile time
constant too. It does in a little userspace test I just wrote, but I
didn't look at its behaviour with the kernel's strlen.
Ian.
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
2012-07-30 14:26 ` Ian Campbell
@ 2012-07-30 14:41 ` richard -rw- weinberger
0 siblings, 0 replies; 29+ messages in thread
From: richard -rw- weinberger @ 2012-07-30 14:41 UTC (permalink / raw)
To: Ian Campbell
Cc: Len Brown, Alan Stern, linux-acpi, linux-pm, Pavel Vasilyev,
linux-kernel, Len Brown
On Mon, Jul 30, 2012 at 4:26 PM, Ian Campbell <ijc@hellion.org.uk> wrote:
> I thought gcc optimised strlen("string literal") into a compile time
> constant too. It does in a little userspace test I just wrote, but I
> didn't look at its behaviour with the kernel's strlen.
It depends whether you use the built-in strlen() or not.
AFAIK the kernel does not use the built-in variant.
--
Thanks,
//richard
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [linux-pm] [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
2012-07-27 1:39 ` [PATCH] ACPI: replace strlen("string") with sizeof("string") -1 Len Brown
2012-07-30 14:26 ` Ian Campbell
@ 2012-08-06 10:21 ` Pavel Machek
2012-08-06 14:36 ` Alan Stern
2012-08-06 10:26 ` Andreas Schwab
2 siblings, 1 reply; 29+ messages in thread
From: Pavel Machek @ 2012-08-06 10:21 UTC (permalink / raw)
To: Len Brown
Cc: Alan Stern, linux-acpi, linux-pm, linux-kernel, Len Brown,
Pavel Vasilyev
On Thu 2012-07-26 21:39:38, Len Brown wrote:
> ...both give the number of chars in the string
> without the '\0', as strncmp() wants,
> but sizeof() is compile-time.
What about introducing something like streq() to do this
automatically? This is ugly....
#define streq(a, b) ... if (_buildin_constant(b)) ...
?
> - if (!strncmp(val, "enable", strlen("enable"))) {
> + if (!strncmp(val, "enable", sizeof("enable") - 1)) {
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
2012-08-06 10:21 ` [linux-pm] " Pavel Machek
@ 2012-08-06 14:36 ` Alan Stern
2012-08-06 16:00 ` [linux-pm] " Pavel Vasilyev
0 siblings, 1 reply; 29+ messages in thread
From: Alan Stern @ 2012-08-06 14:36 UTC (permalink / raw)
To: Pavel Machek
Cc: Len Brown, linux-kernel, linux-acpi, Pavel Vasilyev, linux-pm
On Mon, 6 Aug 2012, Pavel Machek wrote:
> On Thu 2012-07-26 21:39:38, Len Brown wrote:
> > ...both give the number of chars in the string
> > without the '\0', as strncmp() wants,
> > but sizeof() is compile-time.
>
> What about introducing something like streq() to do this
> automatically? This is ugly....
>
> #define streq(a, b) ... if (_buildin_constant(b)) ...
>
> ?
>
> > - if (!strncmp(val, "enable", strlen("enable"))) {
> > + if (!strncmp(val, "enable", sizeof("enable") - 1)) {
While you're at it, there's no point using strncmp when you know the
length of one of the strings beforehand. Just use memcmp, and don't
subtract 1 from the sizeof value.
Alan Stern
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [linux-pm] [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
2012-08-06 14:36 ` Alan Stern
@ 2012-08-06 16:00 ` Pavel Vasilyev
2012-08-06 16:28 ` Alan Stern
0 siblings, 1 reply; 29+ messages in thread
From: Pavel Vasilyev @ 2012-08-06 16:00 UTC (permalink / raw)
To: Alan Stern
Cc: Pavel Machek, Len Brown, linux-acpi, linux-pm, linux-kernel,
Len Brown
06.08.2012 18:36, Alan Stern пишет:
> On Mon, 6 Aug 2012, Pavel Machek wrote:
>
>> On Thu 2012-07-26 21:39:38, Len Brown wrote:
>>> ...both give the number of chars in the string
>>> without the '\0', as strncmp() wants,
>>> but sizeof() is compile-time.
>>
>> What about introducing something like streq() to do this
>> automatically? This is ugly....
>>
>> #define streq(a, b) ... if (_buildin_constant(b)) ...
>>
>> ?
>>
>>> - if (!strncmp(val, "enable", strlen("enable"))) {
>>> + if (!strncmp(val, "enable", sizeof("enable") - 1)) {
>
> While you're at it, there's no point using strncmp when you know the
> length of one of the strings beforehand. Just use memcmp, and don't
> subtract 1 from the sizeof value.
http://www.gossamer-threads.com/lists/engine?do=post_attachment;postatt_id=41157;list=linux
:)
--
Pavel.
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
2012-08-06 16:00 ` [linux-pm] " Pavel Vasilyev
@ 2012-08-06 16:28 ` Alan Stern
2012-08-06 18:47 ` [linux-pm] " Pavel Vasilyev
0 siblings, 1 reply; 29+ messages in thread
From: Alan Stern @ 2012-08-06 16:28 UTC (permalink / raw)
To: Pavel Vasilyev; +Cc: Len Brown, linux-kernel, linux-acpi, linux-pm
On Mon, 6 Aug 2012, Pavel Vasilyev wrote:
> 06.08.2012 18:36, Alan Stern пишет:
> > On Mon, 6 Aug 2012, Pavel Machek wrote:
> >
> >> On Thu 2012-07-26 21:39:38, Len Brown wrote:
> >>> ...both give the number of chars in the string
> >>> without the '\0', as strncmp() wants,
> >>> but sizeof() is compile-time.
> >>
> >> What about introducing something like streq() to do this
> >> automatically? This is ugly....
> >>
> >> #define streq(a, b) ... if (_buildin_constant(b)) ...
> >>
> >> ?
> >>
> >>> - if (!strncmp(val, "enable", strlen("enable"))) {
> >>> + if (!strncmp(val, "enable", sizeof("enable") - 1)) {
> >
> > While you're at it, there's no point using strncmp when you know the
> > length of one of the strings beforehand. Just use memcmp, and don't
> > subtract 1 from the sizeof value.
>
> http://www.gossamer-threads.com/lists/engine?do=post_attachment;postatt_id=41157;list=linux
Interestingly, many (all?) of the changes in that patch are wrong
because they don't try to match the terminating '\0'. As a result,
they will match against extensions of the target string as well as the
target string itself.
Alan Stern
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [linux-pm] [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
2012-08-06 16:28 ` Alan Stern
@ 2012-08-06 18:47 ` Pavel Vasilyev
2012-08-06 19:59 ` Alan Stern
0 siblings, 1 reply; 29+ messages in thread
From: Pavel Vasilyev @ 2012-08-06 18:47 UTC (permalink / raw)
To: Alan Stern
Cc: Pavel Machek, Len Brown, linux-acpi, linux-pm, linux-kernel,
Len Brown
06.08.2012 20:28, Alan Stern пишет:
> On Mon, 6 Aug 2012, Pavel Vasilyev wrote:
>
>> 06.08.2012 18:36, Alan Stern пишет:
>>> On Mon, 6 Aug 2012, Pavel Machek wrote:
>>>
>>>> On Thu 2012-07-26 21:39:38, Len Brown wrote:
>>>>> ...both give the number of chars in the string
>>>>> without the '\0', as strncmp() wants,
>>>>> but sizeof() is compile-time.
>>>>
>>>> What about introducing something like streq() to do this
>>>> automatically? This is ugly....
>>>>
>>>> #define streq(a, b) ... if (_buildin_constant(b)) ...
>>>>
>>>> ?
>>>>
>>>>> - if (!strncmp(val, "enable", strlen("enable"))) {
>>>>> + if (!strncmp(val, "enable", sizeof("enable") - 1)) {
>>>
>>> While you're at it, there's no point using strncmp when you know the
>>> length of one of the strings beforehand. Just use memcmp, and don't
>>> subtract 1 from the sizeof value.
>>
>> http://www.gossamer-threads.com/lists/engine?do=post_attachment;postatt_id=41157;list=linux
>
> Interestingly, many (all?) of the changes in that patch are wrong
> because they don't try to match the terminating '\0'. As a result,
> they will match against extensions of the target string as well as the
> target string itself.
>
strNcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L270
memcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L651
--
Pavel.
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
2012-08-06 18:47 ` [linux-pm] " Pavel Vasilyev
@ 2012-08-06 19:59 ` Alan Stern
2012-08-06 22:57 ` [linux-pm] " Daniel Taylor
2012-08-07 1:07 ` Pavel Vasilyev
0 siblings, 2 replies; 29+ messages in thread
From: Alan Stern @ 2012-08-06 19:59 UTC (permalink / raw)
To: Pavel Vasilyev; +Cc: Len Brown, linux-kernel, linux-acpi, linux-pm
On Mon, 6 Aug 2012, Pavel Vasilyev wrote:
> >> http://www.gossamer-threads.com/lists/engine?do=post_attachment;postatt_id=41157;list=linux
> >
> > Interestingly, many (all?) of the changes in that patch are wrong
> > because they don't try to match the terminating '\0'. As a result,
> > they will match against extensions of the target string as well as the
> > target string itself.
> >
>
> strNcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L270
> memcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L651
Yes. So if s contains "abcde" then
memcmp(s, "abc", 3) and strncmp(s, "abc", 3) will both return 0, and
memcmp(s, "abc", 4) and strncmp(s, "abc", 4) will both return 1.
Alan Stern
^ permalink raw reply [flat|nested] 29+ messages in thread* RE: [linux-pm] [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
2012-08-06 19:59 ` Alan Stern
@ 2012-08-06 22:57 ` Daniel Taylor
2012-08-07 13:19 ` Bernd Petrovitsch
2012-08-07 1:07 ` Pavel Vasilyev
1 sibling, 1 reply; 29+ messages in thread
From: Daniel Taylor @ 2012-08-06 22:57 UTC (permalink / raw)
To: 'Alan Stern', Pavel Vasilyev
Cc: Pavel Machek, Len Brown, linux-acpi@vger.kernel.org,
linux-pm@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
Len Brown
Silly question: when did sizeof("string") get changed to be anything
other than the size of the pointer ("string" is, after all, an array
of characters)?
> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org
> [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Alan Stern
> Sent: Monday, August 06, 2012 1:00 PM
> To: Pavel Vasilyev
> Cc: Pavel Machek; Len Brown; linux-acpi@vger.kernel.org;
> linux-pm@lists.linux-foundation.org;
> linux-kernel@vger.kernel.org; Len Brown
> Subject: Re: [linux-pm] [PATCH] ACPI: replace
> strlen("string") with sizeof("string") -1
>
> On Mon, 6 Aug 2012, Pavel Vasilyev wrote:
>
> > >>
> http://www.gossamer-threads.com/lists/engine?do=post_attachmen
> t;postatt_id=41157;list=linux
> > >
> > > Interestingly, many (all?) of the changes in that patch are wrong
> > > because they don't try to match the terminating '\0'. As
> a result,
> > > they will match against extensions of the target string
> as well as the
> > > target string itself.
> > >
> >
> > strNcmp compare N bytes -
> http://lxr.linux.no/#linux+v3.5/lib/string.c#L270
> > memcmp compare N bytes -
> http://lxr.linux.no/#linux+v3.5/lib/string.c#L651
>
> Yes. So if s contains "abcde" then
>
> memcmp(s, "abc", 3) and strncmp(s, "abc", 3) will both
> return 0, and
> memcmp(s, "abc", 4) and strncmp(s, "abc", 4) will both return 1.
>
> Alan Stern
>
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
2012-08-06 22:57 ` [linux-pm] " Daniel Taylor
@ 2012-08-07 13:19 ` Bernd Petrovitsch
2012-08-08 0:59 ` [linux-pm] " Daniel Taylor
0 siblings, 1 reply; 29+ messages in thread
From: Bernd Petrovitsch @ 2012-08-07 13:19 UTC (permalink / raw)
To: Daniel Taylor
Cc: Len Brown, linux-kernel@vger.kernel.org,
linux-acpi@vger.kernel.org, Pavel Vasilyev,
linux-pm@lists.linux-foundation.org
On Mon, 2012-08-06 at 22:57 +0000, Daniel Taylor wrote:
> Silly question: when did sizeof("string") get changed to be anything
> other than the size of the pointer ("string" is, after all, an array
> of characters)?
It is since K&R times that way.
If you do not know the difference between a pointer and an array (and
these are vastly different), go learn something new about C.
Bernd
--
Bernd Petrovitsch Email : bernd@petrovitsch.priv.at
LUGA : http://www.luga.at
^ permalink raw reply [flat|nested] 29+ messages in thread* RE: [linux-pm] [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
2012-08-07 13:19 ` Bernd Petrovitsch
@ 2012-08-08 0:59 ` Daniel Taylor
0 siblings, 0 replies; 29+ messages in thread
From: Daniel Taylor @ 2012-08-08 0:59 UTC (permalink / raw)
To: 'Bernd Petrovitsch'
Cc: 'Alan Stern', Pavel Vasilyev, Pavel Machek, Len Brown,
linux-acpi@vger.kernel.org, linux-pm@lists.linux-foundation.org,
linux-kernel@vger.kernel.org, Len Brown
Said it was a silly question.
It's funny.
I've been using "0123456789abcdef"[index] for a long time, so I "know"
that "string" is a array of char, but it never occurred to me that
"string" would work in sizeof() the same way as
char string[] = { '0', '1', '2', '3', '4', '5', '6', '7',
'8', '9', 'a', 'b', 'c', 'd', 'e', 'f', '\0' };
int stringlength = sizeof(string);
Learned something.
Thanks,
Dan
> -----Original Message-----
> From: Bernd Petrovitsch [mailto:bernd@petrovitsch.priv.at]
> Sent: Tuesday, August 07, 2012 6:20 AM
> To: Daniel Taylor
> Cc: 'Alan Stern'; Pavel Vasilyev; Pavel Machek; Len Brown;
> linux-acpi@vger.kernel.org;
> linux-pm@lists.linux-foundation.org;
> linux-kernel@vger.kernel.org; Len Brown
> Subject: RE: [linux-pm] [PATCH] ACPI: replace
> strlen("string") with sizeof("string") -1
>
> On Mon, 2012-08-06 at 22:57 +0000, Daniel Taylor wrote:
> > Silly question: when did sizeof("string") get changed to
> be anything
> > other than the size of the pointer ("string" is, after all, an array
> > of characters)?
>
> It is since K&R times that way.
> If you do not know the difference between a pointer and an array (and
> these are vastly different), go learn something new about C.
>
> Bernd
> --
> Bernd Petrovitsch Email : bernd@petrovitsch.priv.at
> LUGA : http://www.luga.at
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [linux-pm] [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
2012-08-06 19:59 ` Alan Stern
2012-08-06 22:57 ` [linux-pm] " Daniel Taylor
@ 2012-08-07 1:07 ` Pavel Vasilyev
2012-08-07 17:24 ` Alan Stern
1 sibling, 1 reply; 29+ messages in thread
From: Pavel Vasilyev @ 2012-08-07 1:07 UTC (permalink / raw)
To: Alan Stern
Cc: Pavel Machek, Len Brown, linux-acpi, linux-pm, linux-kernel,
Len Brown
06.08.2012 23:59, Alan Stern пишет:
> On Mon, 6 Aug 2012, Pavel Vasilyev wrote:
>
>>>> http://www.gossamer-threads.com/lists/engine?do=post_attachment;postatt_id=41157;list=linux
>>>
>>> Interestingly, many (all?) of the changes in that patch are wrong
>>> because they don't try to match the terminating '\0'. As a result,
>>> they will match against extensions of the target string as well as the
>>> target string itself.
>>>
>>
>> strNcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L270
>> memcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L651
>
> Yes. So if s contains "abcde" then
>
> memcmp(s, "abc", 3) and strncmp(s, "abc", 3) will both return 0, and
> memcmp(s, "abc", 4) and strncmp(s, "abc", 4) will both return 1.
No matter what is contained in *s, "abcde" or "abcxxx",
are important first N bytes. The second example, you see,
a little bit stupid, and devoid of logic. :)
--
Pavel.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
2012-08-07 1:07 ` Pavel Vasilyev
@ 2012-08-07 17:24 ` Alan Stern
2012-08-07 23:23 ` [linux-pm] " Pavel Vasilyev
0 siblings, 1 reply; 29+ messages in thread
From: Alan Stern @ 2012-08-07 17:24 UTC (permalink / raw)
To: Pavel Vasilyev; +Cc: Len Brown, linux-kernel, linux-acpi, linux-pm
On Tue, 7 Aug 2012, Pavel Vasilyev wrote:
> 06.08.2012 23:59, Alan Stern пишет:
> > On Mon, 6 Aug 2012, Pavel Vasilyev wrote:
> >
> >>>> http://www.gossamer-threads.com/lists/engine?do=post_attachment;postatt_id=41157;list=linux
> >>>
> >>> Interestingly, many (all?) of the changes in that patch are wrong
> >>> because they don't try to match the terminating '\0'. As a result,
> >>> they will match against extensions of the target string as well as the
> >>> target string itself.
> >>>
> >>
> >> strNcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L270
> >> memcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L651
> >
> > Yes. So if s contains "abcde" then
> >
> > memcmp(s, "abc", 3) and strncmp(s, "abc", 3) will both return 0, and
> > memcmp(s, "abc", 4) and strncmp(s, "abc", 4) will both return 1.
>
> No matter what is contained in *s, "abcde" or "abcxxx",
> are important first N bytes. The second example, you see,
> a little bit stupid, and devoid of logic. :)
Maybe yes, maybe no. It all depends on what you want.
For example, if you're looking for "on" or "off", what should you do
when the user writes "onoff"? You could accept it as meaning the same
as "on", but if you were being careful then you would want to reject it
as a meaningless value.
Alan Stern
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [linux-pm] [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
2012-08-07 17:24 ` Alan Stern
@ 2012-08-07 23:23 ` Pavel Vasilyev
2012-08-08 1:27 ` Alan Stern
0 siblings, 1 reply; 29+ messages in thread
From: Pavel Vasilyev @ 2012-08-07 23:23 UTC (permalink / raw)
To: Alan Stern
Cc: Pavel Machek, Len Brown, linux-acpi, linux-pm, linux-kernel,
Len Brown
07.08.2012 21:24, Alan Stern пишет:
> On Tue, 7 Aug 2012, Pavel Vasilyev wrote:
>
>> 06.08.2012 23:59, Alan Stern пишет:
>>> On Mon, 6 Aug 2012, Pavel Vasilyev wrote:
>>>
>>>>>> http://www.gossamer-threads.com/lists/engine?do=post_attachment;postatt_id=41157;list=linux
>>>>>
>>>>> Interestingly, many (all?) of the changes in that patch are wrong
>>>>> because they don't try to match the terminating '\0'. As a result,
>>>>> they will match against extensions of the target string as well as the
>>>>> target string itself.
>>>>>
>>>>
>>>> strNcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L270
>>>> memcmp compare N bytes - http://lxr.linux.no/#linux+v3.5/lib/string.c#L651
>>>
>>> Yes. So if s contains "abcde" then
>>>
>>> memcmp(s, "abc", 3) and strncmp(s, "abc", 3) will both return 0, and
>>> memcmp(s, "abc", 4) and strncmp(s, "abc", 4) will both return 1.
>>
>> No matter what is contained in *s, "abcde" or "abcxxx",
>> are important first N bytes. The second example, you see,
>> a little bit stupid, and devoid of logic. :)
>
> Maybe yes, maybe no. It all depends on what you want.
>
> For example, if you're looking for "on" or "off", what should you do
> when the user writes "onoff"? You could accept it as meaning the same
> as "on", but if you were being careful then you would want to reject it
> as a meaningless value.
The users should't be allowed to think!
There is "on" - the size of 2 bytes, or "off" - 3 bytes,
other variations - user error.
We do not create a kernel with artificial intelligence? ;)
--
Pavel.
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [linux-pm] [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
2012-08-07 23:23 ` [linux-pm] " Pavel Vasilyev
@ 2012-08-08 1:27 ` Alan Stern
0 siblings, 0 replies; 29+ messages in thread
From: Alan Stern @ 2012-08-08 1:27 UTC (permalink / raw)
To: Pavel Vasilyev
Cc: Pavel Machek, Len Brown, linux-acpi, linux-pm, linux-kernel,
Len Brown
On Wed, 8 Aug 2012, Pavel Vasilyev wrote:
> >>> Yes. So if s contains "abcde" then
> >>>
> >>> memcmp(s, "abc", 3) and strncmp(s, "abc", 3) will both return 0, and
> >>> memcmp(s, "abc", 4) and strncmp(s, "abc", 4) will both return 1.
> >>
> >> No matter what is contained in *s, "abcde" or "abcxxx",
> >> are important first N bytes. The second example, you see,
> >> a little bit stupid, and devoid of logic. :)
> >
> > Maybe yes, maybe no. It all depends on what you want.
> >
> > For example, if you're looking for "on" or "off", what should you do
> > when the user writes "onoff"? You could accept it as meaning the same
> > as "on", but if you were being careful then you would want to reject it
> > as a meaningless value.
>
>
> The users should't be allowed to think!
> There is "on" - the size of 2 bytes, or "off" - 3 bytes,
> other variations - user error.
>
> We do not create a kernel with artificial intelligence? ;)
Let me rephrase the previous statement, as it appears you did not
understand what I meant.
If the kernel is testing for "on" or "off", what should it do when the
user writes "onoff"? The kernel could accept this as meaning the same
as "on", but if the kernel was being careful then it should reject
"onoff" as a meaningless value. A 2-byte comparison for "on" would
accept "onoff" whereas a 3-byte comparison would not.
Alan Stern
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] ACPI: replace strlen("string") with sizeof("string") -1
2012-07-27 1:39 ` [PATCH] ACPI: replace strlen("string") with sizeof("string") -1 Len Brown
2012-07-30 14:26 ` Ian Campbell
2012-08-06 10:21 ` [linux-pm] " Pavel Machek
@ 2012-08-06 10:26 ` Andreas Schwab
2 siblings, 0 replies; 29+ messages in thread
From: Andreas Schwab @ 2012-08-06 10:26 UTC (permalink / raw)
To: Len Brown
Cc: Alan Stern, linux-acpi, linux-pm, linux-kernel, Len Brown,
Pavel Vasilyev
Len Brown <lenb@kernel.org> writes:
> ...both give the number of chars in the string
> without the '\0', as strncmp() wants,
> but sizeof() is compile-time.
Does this actually change anything? The compiler is able to expand
strlen at compile time if the argument is a constant, provided that that
builtin strlen isn't disabled.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 29+ messages in thread