4:11 p.m.
Hello!
Sometimes PowerTOP dies with a Segmentation Fault while generating
the report. Little investigation shown that it dies while sorting
the device list. The problem is that comparison function devlist_sort
is incorrect.
The function should return true if first argument "preceeds" second.
But when strcmp() is used, it can return -1, 0 and 1. So both -1 and 1
values are silently converted to true, which is wrong. It confuses the sort()
function and it crosses boundary of array.
Here is the patch:
--8<--Cut-here----------------------------------------------------------------
diff -purN powertop-il/src/devlist.cpp powertop-il-fix/src/devlist.cpp
--- powertop-il/src/devlist.cpp 2012-07-03 17:00:50.000000000 +0400
+++ powertop-il-fix/src/devlist.cpp 2012-07-03 20:01:05.896232845 +0400
@@ -273,7 +273,7 @@ static bool devlist_sort(struct devuser
if (i->pid != j->pid)
return i->pid< j->pid;
- return strcmp(i->device, j->device);
+ return (strcmp(i->device, j->device)< 0);
}
static const char *dev_class(int line)
--8<--------------------------------------------------------------------------
Thank you.
--
Best regards,
Igor Zhbanov,
Expert Software Engineer,
phone: +7 (495) 797 25 00 ext 3806
e-mail: i.zhbanov(a)samsung.com
ASWG, Moscow R&D center, Samsung Electronics
12 Dvintsev street, building 1
127018, Moscow, Russian Federation
5:33 p.m.
New subject: [Powertop] Device list sort function bug
On (07/11/12 10:11), Igor Zhbanov wrote:
Sometimes PowerTOP dies with a Segmentation Fault while generating
the report. Little investigation shown that it dies while sorting
the device list. The problem is that comparison function devlist_sort
is incorrect.
The function should return true if first argument "preceeds" second.
But when strcmp() is used, it can return -1, 0 and 1. So both -1 and 1
values are silently converted to true, which is wrong. It confuses the sort()
function and it crosses boundary of array.
Jan, could you please test this one?
Thanks in advance,
-ss
5:52 p.m.
New subject: [Powertop] Device list sort function bug
Dear Igor,
thank you for your patches!
Am Mittwoch, den 11.07.2012, 10:11 +0400 schrieb Igor Zhbanov:
Sometimes PowerTOP dies with a Segmentation Fault while generating
the report. Little investigation shown that it dies while sorting
the device list. The problem is that comparison function devlist_sort
is incorrect.
The function should return true if first argument "preceeds" second.
But when strcmp() is used, it can return -1, 0 and 1. So both -1 and 1
values are silently converted to true, which is wrong. It confuses the sort()
function and it crosses boundary of array.
Here is the patch:
--8<--Cut-here----------------------------------------------------------------
diff -purN powertop-il/src/devlist.cpp powertop-il-fix/src/devlist.cpp
--- powertop-il/src/devlist.cpp 2012-07-03 17:00:50.000000000 +0400
+++ powertop-il-fix/src/devlist.cpp 2012-07-03 20:01:05.896232845 +0400
@@ -273,7 +273,7 @@ static bool devlist_sort(struct devuser
if (i->pid != j->pid)
return i->pid< j->pid;
- return strcmp(i->device, j->device);
+ return (strcmp(i->device, j->device)< 0);
}
static const char *dev_class(int line)
--8<--------------------------------------------------------------------------
Nice find!
To decrease work for the maintainers, could you please send patches
formatted by `git format-patch`? After that you can either use `git
send-email` or copy the output into Seamonkey’s new message window.
$ git clone git://github.com/fenrus75/powertop.git
$ cd powertop
$ git config --global user.name "Igor Zhbanov"
$ git config --global user.email i.zhbanov(a)samsung.com
$ git checkout -b sort-fix # new branch for each fix
$ # apply your change
$ git commit -a # enter a nice commit message \
you should be able to use your great ones from your messages
$ git format-patch -1
Then maintainers can just apply it using the following command.
$ git am yourmessagewithpatch.mbox
If you need more help with Git, just ask.
Thanks,
Paul
5:55 p.m.
New subject: [Powertop] Device list sort function bug
Yes, in an hour or so.
Best,
JS
Am Mittwoch, 11. Juli 2012, 09:33:42 schrieb Sergey Senozhatsky:
On (07/11/12 10:11), Igor Zhbanov wrote:
> Sometimes PowerTOP dies with a Segmentation Fault while generating
> the report. Little investigation shown that it dies while sorting
> the device list. The problem is that comparison function devlist_sort
> is incorrect.
>
> The function should return true if first argument "preceeds" second.
> But when strcmp() is used, it can return -1, 0 and 1. So both -1 and 1
> values are silently converted to true, which is wrong. It confuses the
> sort() function and it crosses boundary of array.
Jan, could you please test this one?
Thanks in advance,
-ss
6:16 p.m.
New subject: [Powertop] Device list sort function bug
So here is the formatted version:
From 5d284137a6035bab65717fbacd98423b96d862cf Mon Sep 17 00:00:00 2001
From: Igor Zhbanov<i.zhbanov(a)samsung.com>
Date: Wed, 11 Jul 2012 12:08:21 +0400
Subject: [PATCH] Device list sort function bug
Sometimes PowerTOP dies with a Segmentation Fault while generating
the report. Little investigation shown that it dies while sorting
the device list. The problem is that comparison function devlist_sort
is incorrect.
The function should return true if first argument "preceeds" second.
But when strcmp() is used, it can return -1, 0 and 1. So both -1 and 1
values are silently converted to true, which is wrong. It confuses the sort()
function and it crosses boundary of array.
---
src/devlist.cpp | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/devlist.cpp b/src/devlist.cpp
index 93f2081..cd5b5d8 100644
--- a/src/devlist.cpp
+++ b/src/devlist.cpp
@@ -273,7 +273,7 @@ static bool devlist_sort(struct devuser * i, struct devuser * j)
if (i->pid != j->pid)
return i->pid< j->pid;
- return strcmp(i->device, j->device);
+ return (strcmp(i->device, j->device)< 0);
}
static const char *dev_class(int line)
--
1.7.5.4
Jan-Simon Möller wrote:
Yes, in an hour or so.
Best,
JS
Am Mittwoch, 11. Juli 2012, 09:33:42 schrieb Sergey Senozhatsky:
> On (07/11/12 10:11), Igor Zhbanov wrote:
>> Sometimes PowerTOP dies with a Segmentation Fault while generating
>> the report. Little investigation shown that it dies while sorting
>> the device list. The problem is that comparison function devlist_sort
>> is incorrect.
>>
>> The function should return true if first argument "preceeds" second.
>> But when strcmp() is used, it can return -1, 0 and 1. So both -1 and 1
>> values are silently converted to true, which is wrong. It confuses the
>> sort() function and it crosses boundary of array.
> Jan, could you please test this one?
>
> Thanks in advance,
>
> -ss
--
Best regards,
Igor Zhbanov,
Expert Software Engineer,
phone: +7 (495) 797 25 00 ext 3806
e-mail: i.zhbanov(a)samsung.com
ASWG, Moscow R&D center, Samsung Electronics
12 Dvintsev street, building 1
127018, Moscow, Russian Federation
6:57 p.m.
New subject: [Powertop] Device list sort function bug
Dear Igor,
thank you for following my advise. Some more minor suggestions are
inlined but until the maintainers ask for further changes you do not
need to take any further actions.
Am Mittwoch, den 11.07.2012, 12:16 +0400 schrieb Igor Zhbanov:
So here is the formatted version:
Normally when sending patches no introduction or email style formalities
(like greeting and so on) are needed.
If you want to use them put them at the top and separate them from the
patch using
---- 8< ----- >8 ----
so that `git am --scissors` strips the email text off automatically.
From 5d284137a6035bab65717fbacd98423b96d862cf Mon Sep 17 00:00:00
2001
From: Igor Zhbanov<i.zhbanov(a)samsung.com>
SeaMonkey and some Thunderbird versions seem to have a bug deleting a
space before »<«.
But if your email account you sending the message from is the same as
the commit author of the patch you can also delete that line.
Date: Wed, 11 Jul 2012 12:08:21 +0400
Subject: [PATCH] Device list sort function bug
You can copy the text after »Subject: « into the email message subject
line. Then your message automatically has the »PATCH« tag.
Sometimes PowerTOP dies with a Segmentation Fault while generating
the report. Little investigation shown that it dies while sorting
the device list. The problem is that comparison function devlist_sort
is incorrect.
The function should return true if first argument "preceeds" second.
But when strcmp() is used, it can return -1, 0 and 1. So both -1 and 1
values are silently converted to true, which is wrong. It confuses the sort()
function and it crosses boundary of array.
---
Some comments not meant for the commit message, like what changed in
patch iterations go after the --- line and Git will ignore them.
src/devlist.cpp | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/devlist.cpp b/src/devlist.cpp
index 93f2081..cd5b5d8 100644
--- a/src/devlist.cpp
+++ b/src/devlist.cpp
@@ -273,7 +273,7 @@ static bool devlist_sort(struct devuser * i, struct devuser * j)
if (i->pid != j->pid)
return i->pid< j->pid;
- return strcmp(i->device, j->device);
+ return (strcmp(i->device, j->device)< 0);
}
static const char *dev_class(int line)
--
1.7.5.4
Just delete the quote next time if you are not citing it in any way [1].
Jan-Simon Möller wrote:
> Yes, in an hour or so.
> Best,
> JS
> Am Mittwoch, 11. Juli 2012, 09:33:42 schrieb Sergey Senozhatsky:
>> On (07/11/12 10:11), Igor Zhbanov wrote:
>>> Sometimes PowerTOP dies with a Segmentation Fault while generating
>>> the report. Little investigation shown that it dies while sorting
>>> the device list. The problem is that comparison function devlist_sort
>>> is incorrect.
>>>
>>> The function should return true if first argument "preceeds"
second.
>>> But when strcmp() is used, it can return -1, 0 and 1. So both -1 and 1
>>> values are silently converted to true, which is wrong. It confuses the
>>> sort() function and it crosses boundary of array.
>> Jan, could you please test this one?
>>
>> Thanks in advance,
>>
>> -ss
Thanks again,
Paul
[1] http://en.opensuse.org/openSUSE:Mailing_list_netiquette
8:09 a.m.
New subject: [Powertop] Device list sort function bug
On 07/10/2012 11:11 PM, Igor Zhbanov wrote:
Hello!
Sometimes PowerTOP dies with a Segmentation Fault while generating
the report. Little investigation shown that it dies while sorting
the device list. The problem is that comparison function devlist_sort
is incorrect.
The function should return true if first argument "preceeds" second.
But when strcmp() is used, it can return -1, 0 and 1. So both -1 and 1
values are silently converted to true, which is wrong. It confuses the
sort()
function and it crosses boundary of array.
Here is the patch:
--8<--Cut-here----------------------------------------------------------------
diff -purN powertop-il/src/devlist.cpp powertop-il-fix/src/devlist.cpp
--- powertop-il/src/devlist.cpp 2012-07-03 17:00:50.000000000 +0400
+++ powertop-il-fix/src/devlist.cpp 2012-07-03 20:01:05.896232845
+0400
@@ -273,7 +273,7 @@ static bool devlist_sort(struct devuser
if (i->pid != j->pid)
return i->pid< j->pid;
- return strcmp(i->device, j->device);
+ return (strcmp(i->device, j->device)< 0);
}
static const char *dev_class(int line)
--8<--------------------------------------------------------------------------
Thank you.
_______________________________________________
PowerTop mailing list
PowerTop(a)lists.01.org
https://lists.01.org/mailman/listinfo/powertop
Your patch has been merged.
Thank you,
3715
days inactive
3721
days old
6 comments
5 participants
participants (5)
-
Chris Ferron -
Igor Zhbanov -
Jan-Simon Möller -
Paul Menzel -
Sergey Senozhatsky