ll_schedule.c: fix "uninitialized task_take" warning in CONFIG_TRACEV

[marc-hb]

Looks like no one ever uses tr_dbg()?

Signed-off-by: Marc Herbert marc.herbert@intel.com

[marc-hb]

https://travis-ci.org/github/thesofproject/sof/jobs/764043292 has hit a docker rate limit, no other error.

[keyonjie]

Is this moving from the line 162 needed or we can keep it in line 162 (with NULL initialization added)? If the moving needed, so as other items like wlist/tlist/task?

[marc-hb]

The moving is not required to fix the warning but it's a good opportunity. Why have variables with a wider scope than needed? More complexity, confusion and more chances to leave variables unitialized or wrongly initialized, to shadow, harder to const, easier to misuse, etc.

C99 even relaxed the obsolete requirement for variables to be at the top of the block, C is like any other more modern language now. I digress.

[lgirdwood]

@marc-hb the descision for variable scope at the start of the function was based on the MISRA C safety critical requirments. https://en.wikipedia.org/wiki/MISRA_C. This is needed for use in motor vehicles.
Any MISRA C complience change here would need to be done by the TSC.

[marc-hb]

@lgirdwood I have done some research and asked my favorite MISRA expert and there does not seem to be any MISRA guideline that this PR violates. Can you be more specific?

On the contrary, there are many security guidelines that this PR helps with:
https://wiki.sei.cmu.edu/confluence/display/c/DCL19-C.+Minimize+the+scope+of+variables+and+functions

Variables and functions should be declared in the minimum scope from which all references to the identifier are still possible.

It find it relatively obvious that smaller scopes, lower uninitialized risk and (when possible) more const/immutability help with safety.

[lgirdwood]

@marc-hb I also like limited scope, but we made a descision to follow MISRA C (and this was our understanding at the time). This can be a topic at next TSC. Can you update.

[keyonjie]

@cujomalainey It's actually C99 recommended while breaking C90 per my understanding.

@marc-hb basically I agree with what you change here but as stated in my first comment, since we use C90 style in all other places the single moving of 'task_take' here looks somewhat odd to me.

[cujomalainey]

I knew it broke a C standard, thanks for checking :)

[marc-hb]

it's actually C99 recommended while breaking C90 per my understanding

No, you're confusing:

main()
{
  printf("");
  int a = 0;
}

with:

main()
{
  printf("");
  {
     int a = 0;
  }
}

Only the former fails to compile with:

gcc -ansi -pedantic
warning: ISO C90 forbids mixing declarations and code [-Wdeclaration-after-statement]

The latter compiles just fine. Variables at the start of a block have been possible since forever.

[ujfalusi]

All of the variable declarations at function level used only within the if (atomic_read(&sch->domain->total_num_tasks)) {}

Moving one declaration under to limit the scope does not help readability (it might even make it worst by implying the the variables at top are used elsewhere - they don't) and when it comes to limiting the scope it is not much effective since the only non scope thing is the call:
platform_shared_commit(sch->domain, sizeof(*sch->domain));
at the end.

I time to time prefer to declare locally in a function to limit the scope and in hope for better readability, in this case I don't see why it would help.

[marc-hb]

Thanks @ujfalusi , that makes sense I will keep everything at the top.

[lgirdwood]

Please use "main" as PR target.

[marc-hb]

So to summarize, the recommendation to move the variable at the top of the function is:

• based on some imaginary MISRA rule
• based on some imaginary C90 standard
• confusing start of function with start of block
• violating security rule CERT-C DCL19-C
• making the code slightly less readable and errors like this one more likely to happen

So unless there's some other, serious reason mentioned soon I will resubmit as is to the main branch.

UPDATE: will keep it at the top, thanks @ujfalusi .

[marc-hb]

Re-submitted to main in #3986