linux-kernel - Re: [linux-dvb] Re: DST/BT878 module customization (.. was: Critical points about ...)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1178270614.12651.303.camel@localhost>
Date:	Fri, 04 May 2007 06:23:34 -0300
From:	Mauro Carvalho Chehab <mchehab@...radead.org>
To:	Trent Piepho <xyzzy@...akeasy.org>
Cc:	Markus Rechberger <mrechberger@...il.com>, linux-dvb@...uxtv.org,
	linux-kernel@...r.kernel.org, Manu Abraham <abraham.manu@...il.com>
Subject: Re: [linux-dvb] Re: DST/BT878 module customization
	(..	was:	Critical points about ...)

> > It would be nice, however, to have a patch making dvb_attach more
> > generic, by e.g. having a variant that allows passing another message.
> 
> Only this message is from dvb_attach():
> > DVB: Unable to find symbol dst_attach()
> 
> Is it saying that it cannot load the module that dst_attach() is in (it
> doesn't know what module that is, modprobe knows that).  If you enabled dst
> support and deleted the module, it would be the same.
> 
> If you turn off dvb_attach() and also disable dst, you should instead get
> this message:
> dst_attach: driver disabled by Kconfig
> 
> Maybe that would look nicer with a "DVB:  " prefix?  That would easier if it
> wasn't necessary to update the printk in each boilerplate stub function.  What
> if one macro created these stubs....
> 
> > frontend_init: Could not find a Twinhan DST.
> > dvb-bt8xx: A frontend driver was not found for device 109e/0878 subsystem fbfb/f800
> 
> These two messages are printed by the dvb-bt8xx driver, not by dvb_attach().
> It would be trivial to change of course, but I'm not sure what would be
> pedantically correct for both dst and non-dst based hardware.

Sorry, this is what I meant: to fix the above message. The dvb_attach is
generic enough.

Maybe a more nice text would be something like:
	"Couldn't initialize frontend helper modules for device ..."

since dvb_attach will also print what modules were not loaded.
> 
> > There's an argument against the prototype changes on dst_attach and
> > dst_ca_attach since they aren't frontend.
> 
> The reason I changed that, is the dst_attach() already did return a
> dvb_frontend pointer, it was just inside an enclosing structure.  i.e. what
> existed before:
> 
> {
> 	struct dst_state *state;
> 	state = dst_attach(...);
> 	card->fe = &state->frontend;
> } /* state goes out of scope */
> 
> The frontend is inside the state struct and the state pointer isn't saved
> anywhere.  dvb-bt8xx just saves a frontend pointer from inside the dst state
> and tosses the state pointer away.  So I changed that to:
> 
> 	card->fe = dst_attach(...);

IMO, this made the code cleaner.

-- 
Cheers,
Mauro

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/