Re: brcmsmac: use container_of instead of cast

[Hauke Mehrtens <hauke@hauke-m.de>]

On 07/12/2012 01:20 PM, Dan Carpenter wrote:
> The patch ed1dd81464f5: "brcmsmac: use container_of instead of cast" 
> from Jun 30, 2012, leads to the following Smatch warning:
> drivers/net/wireless/brcm80211/brcmsmac/aiutils.c:543 ai_detach()
> 	 warn: can 'sii' even be NULL?
> 
>    533  /* may be called with core in reset */
>    534  void ai_detach(struct si_pub *sih)
>    535  {
>    536          struct si_info *sii;
>    537  
>    538          struct si_pub *si_local = NULL;
>    539          memcpy(&si_local, &sih, sizeof(struct si_pub **));

I do not get what this line is used for, it looks like unneeded code,
si_local is never read.

>    540  
>    541          sii = container_of(sih, struct si_info, pub);
>    542  
>    543          if (sii == NULL)
>    544                  return;
>    545  
>    546          kfree(sii);
>    547  }
> 
> 
> Smatch complains because container_of() of does pointer math and the
> check for NULL only works when ->pub is first member of the si_info
> struct.

For now pub is the first member of the struct, but the null check should
be done on sih and it should be moved before the container_of line.

> Are you sure you want to free sii and not sih?
Yes sii should be freed, it is allocated in the function ai_attach()
above and there sii is casted to sih, so this only works if pub is the
first member.

> Also kfree() checks for NULL.
> 
> The memcpy() is pointless.
> 
> The white space is not right.  There should be a blank line after the
> variable declarations.
> 
> regards,
> dan carpenter

Hauke