sw_end represents the last software master, sw_start is index of the
first master, so the number of software masters should be
sw_end - sw_start + 1.
Signed-off-by: Chunyan Zhang <[email protected]>
---
drivers/hwtracing/intel_th/sth.c | 2 +-
drivers/hwtracing/stm/core.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/intel_th/sth.c b/drivers/hwtracing/intel_th/sth.c
index 56101c3..28917d7 100644
--- a/drivers/hwtracing/intel_th/sth.c
+++ b/drivers/hwtracing/intel_th/sth.c
@@ -173,7 +173,7 @@ static int intel_th_sw_init(struct sth_device *sth)
sth->stm.sw_start = reg & 0xffff;
sth->stm.sw_end = reg >> 16;
- sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start;
+ sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start + 1;
dev_dbg(sth->dev, "sw_start: %x sw_end: %x masters: %x nchannels: %x\n",
sth->stm.sw_start, sth->stm.sw_end, sth->sw_nmasters,
sth->stm.sw_nchannels);
diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
index 7f7bdb3..cb676f2 100644
--- a/drivers/hwtracing/stm/core.c
+++ b/drivers/hwtracing/stm/core.c
@@ -632,7 +632,7 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data,
if (!stm_data->packet || !stm_data->sw_nchannels)
return -EINVAL;
- nmasters = stm_data->sw_end - stm_data->sw_start;
+ nmasters = stm_data->sw_end - stm_data->sw_start + 1;
stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL);
if (!stm)
return -ENOMEM;
--
1.9.1
Chunyan Zhang <[email protected]> writes:
> sw_end represents the last software master, sw_start is index of the
> first master, so the number of software masters should be
> sw_end - sw_start + 1.
Looks about right, but it needs to be in two separate patches.
> Signed-off-by: Chunyan Zhang <[email protected]>
> ---
> drivers/hwtracing/intel_th/sth.c | 2 +-
> drivers/hwtracing/stm/core.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwtracing/intel_th/sth.c b/drivers/hwtracing/intel_th/sth.c
> index 56101c3..28917d7 100644
> --- a/drivers/hwtracing/intel_th/sth.c
> +++ b/drivers/hwtracing/intel_th/sth.c
> @@ -173,7 +173,7 @@ static int intel_th_sw_init(struct sth_device *sth)
> sth->stm.sw_start = reg & 0xffff;
> sth->stm.sw_end = reg >> 16;
>
> - sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start;
> + sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start + 1;
> dev_dbg(sth->dev, "sw_start: %x sw_end: %x masters: %x nchannels: %x\n",
> sth->stm.sw_start, sth->stm.sw_end, sth->sw_nmasters,
> sth->stm.sw_nchannels);
> diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
> index 7f7bdb3..cb676f2 100644
> --- a/drivers/hwtracing/stm/core.c
> +++ b/drivers/hwtracing/stm/core.c
> @@ -632,7 +632,7 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data,
> if (!stm_data->packet || !stm_data->sw_nchannels)
> return -EINVAL;
>
> - nmasters = stm_data->sw_end - stm_data->sw_start;
> + nmasters = stm_data->sw_end - stm_data->sw_start + 1;
> stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL);
Or even offsetof(struct stm_device, masters[stm_data->sw_end]).
> if (!stm)
> return -ENOMEM;
> --
> 1.9.1
This is a very old version on git, btw. :)
Thanks,
--
Alex
On Fri, Dec 11, 2015 at 3:31 PM, Alexander Shishkin
<[email protected]> wrote:
> Chunyan Zhang <[email protected]> writes:
>
>> sw_end represents the last software master, sw_start is index of the
>> first master, so the number of software masters should be
>> sw_end - sw_start + 1.
>
> Looks about right, but it needs to be in two separate patches.
Will split this patch later.
>
>> Signed-off-by: Chunyan Zhang <[email protected]>
>> ---
>> drivers/hwtracing/intel_th/sth.c | 2 +-
>> drivers/hwtracing/stm/core.c | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hwtracing/intel_th/sth.c b/drivers/hwtracing/intel_th/sth.c
>> index 56101c3..28917d7 100644
>> --- a/drivers/hwtracing/intel_th/sth.c
>> +++ b/drivers/hwtracing/intel_th/sth.c
>> @@ -173,7 +173,7 @@ static int intel_th_sw_init(struct sth_device *sth)
>> sth->stm.sw_start = reg & 0xffff;
>> sth->stm.sw_end = reg >> 16;
>>
>> - sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start;
>> + sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start + 1;
>> dev_dbg(sth->dev, "sw_start: %x sw_end: %x masters: %x nchannels: %x\n",
>> sth->stm.sw_start, sth->stm.sw_end, sth->sw_nmasters,
>> sth->stm.sw_nchannels);
>> diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
>> index 7f7bdb3..cb676f2 100644
>> --- a/drivers/hwtracing/stm/core.c
>> +++ b/drivers/hwtracing/stm/core.c
>> @@ -632,7 +632,7 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data,
>> if (!stm_data->packet || !stm_data->sw_nchannels)
>> return -EINVAL;
>>
>> - nmasters = stm_data->sw_end - stm_data->sw_start;
>> + nmasters = stm_data->sw_end - stm_data->sw_start + 1;
>> stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL);
>
> Or even offsetof(struct stm_device, masters[stm_data->sw_end]).
Ok, I can add this modification in next version, no need to make this
being a separated patch, right?
>
>> if (!stm)
>> return -ENOMEM;
>> --
>> 1.9.1
>
> This is a very old version on git, btw. :)
Didn't notice this before :), I have been using the one that Ubuntu
14.04 provided.
Thanks,
Chunyan
>
> Thanks,
> --
> Alex
On Fri, Dec 11, 2015 at 3:31 PM, Alexander Shishkin
<[email protected]> wrote:
> Chunyan Zhang <[email protected]> writes:
>
>> sw_end represents the last software master, sw_start is index of the
>> first master, so the number of software masters should be
>> sw_end - sw_start + 1.
>
> Looks about right, but it needs to be in two separate patches.
>
>> Signed-off-by: Chunyan Zhang <[email protected]>
>> ---
>> drivers/hwtracing/intel_th/sth.c | 2 +-
>> drivers/hwtracing/stm/core.c | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hwtracing/intel_th/sth.c b/drivers/hwtracing/intel_th/sth.c
>> index 56101c3..28917d7 100644
>> --- a/drivers/hwtracing/intel_th/sth.c
>> +++ b/drivers/hwtracing/intel_th/sth.c
>> @@ -173,7 +173,7 @@ static int intel_th_sw_init(struct sth_device *sth)
>> sth->stm.sw_start = reg & 0xffff;
>> sth->stm.sw_end = reg >> 16;
>>
>> - sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start;
>> + sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start + 1;
>> dev_dbg(sth->dev, "sw_start: %x sw_end: %x masters: %x nchannels: %x\n",
>> sth->stm.sw_start, sth->stm.sw_end, sth->sw_nmasters,
>> sth->stm.sw_nchannels);
>> diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
>> index 7f7bdb3..cb676f2 100644
>> --- a/drivers/hwtracing/stm/core.c
>> +++ b/drivers/hwtracing/stm/core.c
>> @@ -632,7 +632,7 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data,
>> if (!stm_data->packet || !stm_data->sw_nchannels)
>> return -EINVAL;
>>
>> - nmasters = stm_data->sw_end - stm_data->sw_start;
>> + nmasters = stm_data->sw_end - stm_data->sw_start + 1;
>> stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL);
>
> Or even offsetof(struct stm_device, masters[stm_data->sw_end]).
>
This should use 'offsetofend()'.
Thanks,
Chunyan
>> if (!stm)
>> return -ENOMEM;
>> --
>> 1.9.1
>
> This is a very old version on git, btw. :)
>
> Thanks,
> --
> Alex
Chunyan Zhang <[email protected]> writes:
> On Fri, Dec 11, 2015 at 3:31 PM, Alexander Shishkin
> <[email protected]> wrote:
>> Chunyan Zhang <[email protected]> writes:
>>
>>> sw_end represents the last software master, sw_start is index of the
>>> first master, so the number of software masters should be
>>> sw_end - sw_start + 1.
>>
>> Looks about right, but it needs to be in two separate patches.
>>
>>> Signed-off-by: Chunyan Zhang <[email protected]>
>>> ---
>>> drivers/hwtracing/intel_th/sth.c | 2 +-
>>> drivers/hwtracing/stm/core.c | 2 +-
>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/hwtracing/intel_th/sth.c b/drivers/hwtracing/intel_th/sth.c
>>> index 56101c3..28917d7 100644
>>> --- a/drivers/hwtracing/intel_th/sth.c
>>> +++ b/drivers/hwtracing/intel_th/sth.c
>>> @@ -173,7 +173,7 @@ static int intel_th_sw_init(struct sth_device *sth)
>>> sth->stm.sw_start = reg & 0xffff;
>>> sth->stm.sw_end = reg >> 16;
>>>
>>> - sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start;
>>> + sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start + 1;
>>> dev_dbg(sth->dev, "sw_start: %x sw_end: %x masters: %x nchannels: %x\n",
>>> sth->stm.sw_start, sth->stm.sw_end, sth->sw_nmasters,
>>> sth->stm.sw_nchannels);
>>> diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
>>> index 7f7bdb3..cb676f2 100644
>>> --- a/drivers/hwtracing/stm/core.c
>>> +++ b/drivers/hwtracing/stm/core.c
>>> @@ -632,7 +632,7 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data,
>>> if (!stm_data->packet || !stm_data->sw_nchannels)
>>> return -EINVAL;
>>>
>>> - nmasters = stm_data->sw_end - stm_data->sw_start;
>>> + nmasters = stm_data->sw_end - stm_data->sw_start + 1;
>>> stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL);
>>
>> Or even offsetof(struct stm_device, masters[stm_data->sw_end]).
>>
>
> This should use 'offsetofend()'.
No, actually, just scratch my previous comment as it was completely
wrong, just fix the off-by-one. If we were to use offsetof(), it should
rather be of masters[nmasters], but all we need is to fix the off-by-one
right now.
Regards,
--
Alex
On Fri, Dec 11, 2015 at 4:51 PM, Alexander Shishkin
<[email protected]> wrote:
> Chunyan Zhang <[email protected]> writes:
>
>> On Fri, Dec 11, 2015 at 3:31 PM, Alexander Shishkin
>> <[email protected]> wrote:
>>> Chunyan Zhang <[email protected]> writes:
>>>
>>>> sw_end represents the last software master, sw_start is index of the
>>>> first master, so the number of software masters should be
>>>> sw_end - sw_start + 1.
>>>
>>> Looks about right, but it needs to be in two separate patches.
>>>
>>>> Signed-off-by: Chunyan Zhang <[email protected]>
>>>> ---
>>>> drivers/hwtracing/intel_th/sth.c | 2 +-
>>>> drivers/hwtracing/stm/core.c | 2 +-
>>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/hwtracing/intel_th/sth.c b/drivers/hwtracing/intel_th/sth.c
>>>> index 56101c3..28917d7 100644
>>>> --- a/drivers/hwtracing/intel_th/sth.c
>>>> +++ b/drivers/hwtracing/intel_th/sth.c
>>>> @@ -173,7 +173,7 @@ static int intel_th_sw_init(struct sth_device *sth)
>>>> sth->stm.sw_start = reg & 0xffff;
>>>> sth->stm.sw_end = reg >> 16;
>>>>
>>>> - sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start;
>>>> + sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start + 1;
>>>> dev_dbg(sth->dev, "sw_start: %x sw_end: %x masters: %x nchannels: %x\n",
>>>> sth->stm.sw_start, sth->stm.sw_end, sth->sw_nmasters,
>>>> sth->stm.sw_nchannels);
>>>> diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
>>>> index 7f7bdb3..cb676f2 100644
>>>> --- a/drivers/hwtracing/stm/core.c
>>>> +++ b/drivers/hwtracing/stm/core.c
>>>> @@ -632,7 +632,7 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data,
>>>> if (!stm_data->packet || !stm_data->sw_nchannels)
>>>> return -EINVAL;
>>>>
>>>> - nmasters = stm_data->sw_end - stm_data->sw_start;
>>>> + nmasters = stm_data->sw_end - stm_data->sw_start + 1;
>>>> stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL);
>>>
>>> Or even offsetof(struct stm_device, masters[stm_data->sw_end]).
>>>
>>
>> This should use 'offsetofend()'.
>
> No, actually, just scratch my previous comment as it was completely
> wrong, just fix the off-by-one. If we were to use offsetof(), it should
> rather be of masters[nmasters], but all we need is to fix the off-by-one
> right now.
>
Sorry, you may lose me here, what's 'off-by-one' ?
Thanks,
Chunyan
> Regards,
> --
> Alex
Chunyan Zhang <[email protected]> writes:
> On Fri, Dec 11, 2015 at 4:51 PM, Alexander Shishkin
> <[email protected]> wrote:
>> Chunyan Zhang <[email protected]> writes:
>>
>>> On Fri, Dec 11, 2015 at 3:31 PM, Alexander Shishkin
>>> <[email protected]> wrote:
>>>> Chunyan Zhang <[email protected]> writes:
>>>>
>>>>> sw_end represents the last software master, sw_start is index of the
>>>>> first master, so the number of software masters should be
>>>>> sw_end - sw_start + 1.
>>>>
>>>> Looks about right, but it needs to be in two separate patches.
>>>>
>>>>> Signed-off-by: Chunyan Zhang <[email protected]>
>>>>> ---
>>>>> drivers/hwtracing/intel_th/sth.c | 2 +-
>>>>> drivers/hwtracing/stm/core.c | 2 +-
>>>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/hwtracing/intel_th/sth.c b/drivers/hwtracing/intel_th/sth.c
>>>>> index 56101c3..28917d7 100644
>>>>> --- a/drivers/hwtracing/intel_th/sth.c
>>>>> +++ b/drivers/hwtracing/intel_th/sth.c
>>>>> @@ -173,7 +173,7 @@ static int intel_th_sw_init(struct sth_device *sth)
>>>>> sth->stm.sw_start = reg & 0xffff;
>>>>> sth->stm.sw_end = reg >> 16;
>>>>>
>>>>> - sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start;
>>>>> + sth->sw_nmasters = sth->stm.sw_end - sth->stm.sw_start + 1;
>>>>> dev_dbg(sth->dev, "sw_start: %x sw_end: %x masters: %x nchannels: %x\n",
>>>>> sth->stm.sw_start, sth->stm.sw_end, sth->sw_nmasters,
>>>>> sth->stm.sw_nchannels);
>>>>> diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
>>>>> index 7f7bdb3..cb676f2 100644
>>>>> --- a/drivers/hwtracing/stm/core.c
>>>>> +++ b/drivers/hwtracing/stm/core.c
>>>>> @@ -632,7 +632,7 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data,
>>>>> if (!stm_data->packet || !stm_data->sw_nchannels)
>>>>> return -EINVAL;
>>>>>
>>>>> - nmasters = stm_data->sw_end - stm_data->sw_start;
>>>>> + nmasters = stm_data->sw_end - stm_data->sw_start + 1;
>>>>> stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL);
>>>>
>>>> Or even offsetof(struct stm_device, masters[stm_data->sw_end]).
>>>>
>>>
>>> This should use 'offsetofend()'.
>>
>> No, actually, just scratch my previous comment as it was completely
>> wrong, just fix the off-by-one. If we were to use offsetof(), it should
>> rather be of masters[nmasters], but all we need is to fix the off-by-one
>> right now.
>>
>
> Sorry, you may lose me here, what's 'off-by-one' ?
It's an error when the result of your calculation is off by 1 (one too
few or one too many).
https://www.google.com/search?q=off-by-one
Regards,
--
Alex