Re: Copyright issues, do not copy code and add your own copyrights

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

 



Hi,

On 08/14/2012 11:42 AM, Manu Abraham wrote:
Hi,

On Tue, Aug 14, 2012 at 2:51 PM, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
Hi,


On 08/14/2012 11:10 AM, Manu Abraham wrote:

Hi,

The subject line says it.

Please fix the offending Copyright header.

Offending one.

http://git.linuxtv.org/media_tree.git/blob/staging/for_v3.7:/drivers/media/dvb-frontends/stb6100_proc.h

Original one.

http://git.linuxtv.org/media_tree.git/blob/staging/for_v3.7:/drivers/media/dvb-frontends/stb6100_cfg.h


Or even better, get rid of the offending one and add a i2c_gate_ctrl
parameters to the inline
functions defined in stb6100_cfg.h, as this seems a typical case of
unnecessary code-duplication.


i2c_gate_ctrl is not provided by stb6100 hardware, but by the demodulator
used in conjunction such as a stb0899 as can be seen.

Right, I was merely pointing out that the only difference between the
original function wrappers in stb6100_cfg.h and the ones in stb6100_proc.h,
is the calling of the i2c_gate_ctrl frontend-op if defined. So the 2 files
could be merged into one, with the wrappers getting an extra boolean parameter
making them call the frontend-op when that parameter is true.

Note that if the i2c_gate_ctrl frontend-op should always be called when
present then the extra parameter could be omitted.

<snip>

I would also like to point out that things like these are pretty much wrong:

   27         if (&fe->ops)
   28                 frontend_ops = &fe->ops;
   29         if (&frontend_ops->tuner_ops)
   30                 tuner_ops = &frontend_ops->tuner_ops;
   31         if (tuner_ops->get_state) {

The last check de-references tuner_ops, which only is non-NULL if
fe-ops and fe->ops->tuner_ops are non NULL. So either the last check
needs to be:
              if (tuner_ops && tuner_ops->get_state) {

Or we assume that fe-ops and fe->ops->tuner_ops are always non NULL
when this helper gets called and all the previous checks can be removed.


fe->ops is not NULL in any case, when we reach here, but that conditionality
check causes a slight additional delay. The additional check you proposed
presents no harm, though not bringing any new advantage/disadvantage.

Well if we know that fe->ops and fe->ops->tuner_ops are never NULL, then the
if (&fe->ops) and if (&frontend_ops->tuner_ops) are superfluous and should be
removed, on the other hand if we don't know that, then the get_state check should
be:
               if (tuner_ops && tuner_ops->get_state) {

Either know fe->ops and fe->ops->tuner_ops are never NULL and then all checks
should be removed, or we don't know and we should check them in *all* places
where they are used. What we've now is somewhat of the former, and then some of
the latter, which makes no sense at all.

Regards,

Hans





Regards,

Manu

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


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux