[bug report] Coresight: Allocate trace ID after building the path

[bug report] Coresight: Allocate trace ID after building the path

[Dan Carpenter]

Hello Jie Gan,

Commit d87d76d823d1 ("Coresight: Allocate trace ID after building the
path") from Mar 3, 2025 (linux-next), leads to the following Smatch
static checker warning:

	drivers/hwtracing/coresight/coresight-core.c:697 coresight_path_assign_trace_id()
	warn: assigning signed to unsigned: 'path->trace_id = trace_id' 's32min-(-1),1-s32max'

drivers/hwtracing/coresight/coresight-core.c
    681 void coresight_path_assign_trace_id(struct coresight_path *path,
    682                                     enum cs_mode mode)
    683 {
    684         struct coresight_device *sink = coresight_get_sink(path);
    685         struct coresight_node *nd;
    686         int trace_id;
    687 
    688         list_for_each_entry(nd, &path->path_list, link) {
    689                 /* Assign a trace ID to the path for the first device that wants to do it */
    690                 trace_id = coresight_get_trace_id(nd->csdev, mode, sink);
    691 
    692                 /*
    693                  * 0 in this context is that it didn't want to assign so keep searching.
    694                  * Non 0 is either success or fail.
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
True enough...  coresight_etm_get_trace_id() returns -EINVAL on error.

    695                  */
    696                 if (trace_id != 0) {
--> 697                         path->trace_id = trace_id;
                                ^^^^^^^^^^^^^^^^^^^^^^^^^
It doesn't make sense to store -EINVAL in a u8.

    698                         return;
    699                 }
    700         }
    701 }

regards,
dan carpenter

Re: [bug report] Coresight: Allocate trace ID after building the path

[Jie Gan]

On 3/31/2025 10:14 PM, Dan Carpenter wrote:
> Hello Jie Gan,
> 
> Commit d87d76d823d1 ("Coresight: Allocate trace ID after building the
> path") from Mar 3, 2025 (linux-next), leads to the following Smatch
> static checker warning:
> 
> 	drivers/hwtracing/coresight/coresight-core.c:697 coresight_path_assign_trace_id()
> 	warn: assigning signed to unsigned: 'path->trace_id = trace_id' 's32min-(-1),1-s32max'
> 
> drivers/hwtracing/coresight/coresight-core.c
>      681 void coresight_path_assign_trace_id(struct coresight_path *path,
>      682                                     enum cs_mode mode)
>      683 {
>      684         struct coresight_device *sink = coresight_get_sink(path);
>      685         struct coresight_node *nd;
>      686         int trace_id;
>      687
>      688         list_for_each_entry(nd, &path->path_list, link) {
>      689                 /* Assign a trace ID to the path for the first device that wants to do it */
>      690                 trace_id = coresight_get_trace_id(nd->csdev, mode, sink);
>      691
>      692                 /*
>      693                  * 0 in this context is that it didn't want to assign so keep searching.
>      694                  * Non 0 is either success or fail.
>                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> True enough...  coresight_etm_get_trace_id() returns -EINVAL on error.
> 
>      695                  */
>      696                 if (trace_id != 0) {
> --> 697                         path->trace_id = trace_id;
>                                  ^^^^^^^^^^^^^^^^^^^^^^^^^
> It doesn't make sense to store -EINVAL in a u8.

Thanks for reporting.

Jie

> 
>      698                         return;
>      699                 }
>      700         }
>      701 }
> 
> regards,
> dan carpenter