cam: coccinelle generated fix

[imp]

bzero

[imp]

Current coccinelle patch

@@
type T;
identifier V;
expression PATH;
expression PRIO;
@@

T V;
...
- memset(&V, 0, sizeof(V));
...
- xpt_setup_ccb(&V.ccb_h, PATH, PRIO)
+ xpt_setup_stack_ccb(&V.ccb_h, sizeof(V), PATH, PRIO)
@@
type T;
identifier V;
expression PATH;
expression PRIO;
@@

T V;
...
- bzero(&V, sizeof(V));
...
- xpt_setup_ccb(&V.ccb_h, PATH, PRIO)
+ xpt_setup_stack_ccb(&V.ccb_h, sizeof(V), PATH, PRIO)
@@
type T;
identifier V;
expression PATH;
expression PRIO;
@@

T V;
...
- xpt_setup_ccb(&V.ccb_h, PATH, PRIO)
+ xpt_setup_stack_ccb(&V.ccb_h, sizeof(V), PATH, PRIO)

[imp]

better script, but still misses a few things

[imp]

Oh, this works better

// Script to find all the uses of ccb's allocated on the stack and change
// them to use the new setup function which bzeros before doing a normal
// setup of the CCB.
@@
identifier V;
expression PATH;
expression PRIO;
@@

- memset(&V, 0, sizeof(V));
...
- xpt_setup_ccb(&V.ccb_h, PATH, PRIO)
+ xpt_setup_stack_ccb(&V.ccb_h, sizeof(V), PATH, PRIO)
@@
identifier V;
expression PATH;
expression PRIO;
@@

- bzero(&V, sizeof(V));
...
- xpt_setup_ccb(&V.ccb_h, PATH, PRIO)
+ xpt_setup_stack_ccb(&V.ccb_h, sizeof(V), PATH, PRIO)
@@
identifier V;
expression PATH;
expression PRIO;
@@

- xpt_setup_ccb(&V.ccb_h, PATH, PRIO)
+ xpt_setup_stack_ccb(&V.ccb_h, sizeof(V), PATH, PRIO)

updated the patch based on this

[imp]

Found five errors by hand at least, but unsure how to automate it.

sys/cam/ata/ata_da.c

1121–1122

this is a bug, this should be deleted.

sys/cam/cam_xpt.c

502

this is a bug, but we dump the path into into this ccb, and then dump the path into it, then setup things, which is a pattern not otherwise done.

sys/cam/scsi/scsi_target.c

275

also an ordering bug

sys/dev/sym/sym_hipd.c

3428

ordering bug.

3449–3450

hmmm, this one is a bug...

[imp]

Tweaked the script to replace the bzero/memset with the new function.

// Script to find all the uses of ccb's allocated on the stack and change
// them to use the new setup function which bzeros before doing a normal
// setup of the CCB.
@@
identifier V;
expression PATH;
expression PRIO;
@@

- memset(&V, 0, sizeof(V));
+ xpt_setup_stack_ccb(&V.ccb_h, sizeof(V), PATH, PRIO);
...
- xpt_setup_ccb(&V.ccb_h, PATH, PRIO);
@@
identifier V;
expression PATH;
expression PRIO;
@@

- bzero(&V, sizeof(V));
+ xpt_setup_stack_ccb(&V.ccb_h, sizeof(V), PATH, PRIO);
...
- xpt_setup_ccb(&V.ccb_h, PATH, PRIO);
@@
identifier V;
expression PATH;
expression PRIO;
@@

- xpt_setup_ccb(&V.ccb_h, PATH, PRIO);
+ xpt_setup_stack_ccb(&V.ccb_h, sizeof(V), PATH, PRIO);

[imp]

Regen with newer coccinelle patch

[imp]

The latest script still has 2 bugs: ata_da.c (extra memset) and cam_xpt.c (need to create a temp path like is done elsewhere but aren't)

[imp]

run through git clang-format

mostly better enough to prefer this.

[trasz]

How does cocinelle know the CCB is from the stack, as opposed to being allocated dynamically?

[imp]

apply hand fixes

[imp]

In D30353#681908, @trasz wrote:

How does cocinelle know the CCB is from the stack, as opposed to being allocated dynamically?

Because the script uses a variable, not a pointer, to do the matching. This can distinguish between
a pointer that's cast about vs one that's on the stack. My script is a bit primitive, though, in that
it doesn't find xpt_setup_ccb((struct ccb_hdr *)&stack), but I couldn't find any of those in the tree.
Coiccinelle has a fairly sophisticated semantic matching algorithm. It's main use is migrating APIs
in the linux kernel, which is why the output is linux kernel normal form...

[imp]

Free the path correctly