On Tue, 12 May 2015, Daniel Thompson wrote:
> On 12/05/15 13:46, Nicholas Mc Guire wrote:
>> kdb_task_state_string() introduced in the initial commit 5d5314d6795f
>> ("kdb: core for kgdb back end (1 of 2)") returns unsigned long (a bit
>> array of states) but intermediately it is being assigned to a long which
>> make static code checkers unhappy and also does not not help readability
>> (technically there is no reason to use a signed type here).
>>
>> Type-checking coccinelle spatches are being used to locate type mismatches
>> between function signatures and return values in this case it produced:
>> ./kernel/debug/kdb/kdb_support.c:611 WARNING: return of wrong type
>> unsigned long != long
>>
>> Patch was compile tested with x86_64_defconfig + CONFIG_KGDB=y,
>> CONFIG_KGDB_KDB=y
>>
>> Patch is against 4.1-rc3 (localversion-next is -next-20150512)
>>
>> Signed-off-by: Nicholas Mc Guire <[email protected]>
>> ---
>> kernel/debug/kdb/kdb_support.c | 14 +++++++-------
>> 1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> All (5) call-sites of kdb_task_state_string() were checked and all are
>> expecting an unsigned long as the function signature provides - so this
>> change should have no effect as automatic type conversion did not make
>> the signed type visible externally and internally the signed nature was
>> also not in use.
>>
>> Doc fixup: long -> unsigned long only (and some reformatting this caused)
>>
>> diff --git a/kernel/debug/kdb/kdb_support.c b/kernel/debug/kdb/kdb_support.c
>> index d35cc2d..0bb3b81 100644
>> --- a/kernel/debug/kdb/kdb_support.c
>> +++ b/kernel/debug/kdb/kdb_support.c
>> @@ -544,12 +544,12 @@ int kdb_putword(unsigned long addr, unsigned long
>> word, size_t size)
>> * Returns:
>> * Mask for process state.
>> * Notes:
>> - * The mask folds data from several sources into a single long value, so
>> - * be careful not to overlap the bits. TASK_* bits are in the LSB,
>> - * special cases like UNRUNNABLE are in the MSB. As of 2.6.10-rc1 there
>> - * is no overlap between TASK_* and EXIT_* but that may not always be
>> - * true, so EXIT_* bits are shifted left 16 bits before being stored in
>> - * the mask.
>> + * The mask folds data from several sources into a single unsigned long
>> + * value, so be careful not to overlap the bits. TASK_* bits are in
>> the
>> + * LSB, special cases like UNRUNNABLE are in the MSB. As of 2.6.10-rc1
>> + * there is no overlap between TASK_* and EXIT_* but that may not
>> always
>> + * be true, so EXIT_* bits are shifted left 16 bits before being stored
>> + * in the mask.
>
> The ragged alignment here looks like a spurious replacement of tabs with
> spaces.
>
> BTW having the type of the variable int he comment is pointless anyway.
> If you simply remove the type from the comment ("folds data into several
> sources into a single value") then you don't have to reflow the comment
> in the first place.
>
makes sense - will resubmit without the noise around the doc fix.
>> */
>>
>> /* unrunnable is < 0 */
>> @@ -560,7 +560,7 @@ int kdb_putword(unsigned long addr, unsigned long word,
>> size_t size)
>>
>> unsigned long kdb_task_state_string(const char *s)
>> {
>> - long res = 0;
>> + unsigned long res = 0;
>> if (!s) {
>> s = kdbgetenv("PS");
>> if (!s)
>>
>
> Reviewed-by: Daniel Thompson <[email protected]>
>
thx!
hofrat
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
