Re: [patch 2/6] usb: gadget: uvc: cleanup __uvcg_fill_strm()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



W dniu 14.01.2015 o 22:01, Dan Carpenter pisze:
Static checkers complain about this API:

	drivers/usb/gadget/function/uvc_configfs.c:2139
		uvcg_streaming_class_allow_link()
	warn: did you really mean to pass the address of 'data'?

Indeed, the code is cleaner when we just pass the pointer instead of the
pointer to the pointer.

Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>

I'm sorry but this patch breaks the code.

You should look at the broader context in which the __uvcg_fill_strm()
is called: it is used as a callback from __uvcg_iter_strm_cls().
The purpose of the latter is to iterate over a hierarchy of streaming
descriptors' config items built by the user with configfs.
This function is generic, because it needs to be called twice:
the first time to calculate the actual amount of memory needed for
an array of descriptors which is to be created, and the second time
to actually fill the said array - this is the purpose of __uvcg_fill_strm().

__uvcg_iter_strm_cls() flow of control is more or less like this:
"process" the header, then for each format that follows the header
"process" the format itself and then for each frame inside
a format "process" the frame.

__uvcg_fill_stream(), which is used to "process", during its execution
uses priv2 as a pointer to a pointer, because it advances the current
position by the amount of data taken by each processed descriptor
and the advanced position should be visible outside this function,
so that the next time it is called, the current position corresponds
to the place where it was the last time rather than again at
the beginning of some block of data. In other words priv2
is an "out" parameter.

---
Looks obvious enough to me, but I've only compiled this code and haven't
tested it.


What I suggest is to revert this patch and then I will post another
patch which adds comments to __uvcg_iterm_strm_cls(), __uvcg_cnt_strm()
and __uvcg_fill_strm() so that their purpose and meaning of parameters
is clear.

AP
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux