[lkml]   [2007]   [Jul]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [git patches] two warning fixes
    On Fri, 2007-07-20 at 20:34 +0200, Krzysztof Halasa wrote:
    > Linus Torvalds <> writes:
    > > More people *should* generally ask themselves: "was the warning worth it?"
    > > and then, if the answer is "no", they shouldn't add code, they should
    > > remove the thing that causes the warning in the first place.
    > Sure. If a routine uses must_check yet its return value may be
    > safely ignored then that must_check is simply misplaced and should
    > be removed. It does not mean all must_checks are bad - each of them
    > isn't bad unless one can demonstrate it is.
    > Back to sysfs_create_bin_file() - if one can demonstrate a caller
    > can safely ignore the return value (which, it seems, is the
    > case), then exactly this very must_check should be removed

    Typically, the EDID creation in radeonfb :-)

    In fact, I'm not even sure there's -any- user of those sysfs files. I
    added them back then to allow distros to extract the EDID infos that
    were probed by radeonfb to properly configure the X server (because on
    some machines, the EDID is coming from the firmware/BIOS, not from DDC,
    and X can't get at it). I don't know if they ever used them.

    In any case, it doesn't make sense to abort initialization of the driver
    if for some reasons those files can't be created (for example, the core
    fbdev starts exposing EDID files, radeonfb isn't properly updated, name
    clash, error). Aborting the initialization will make sure that on some
    machines such as powermacs with radeon, whatever error is displayed will
    never be seen by the user.

    That's a typical, but I have plenty more.

    For example, the powermac thermal control drivers. They work pretty well
    by themselves. They also expose via sysfs all the current values, fan
    speeds, temps ,etc... for the sake of whoever wants to do a GUI or
    "monitor" what's going on, but that is not critical to the operation of
    the driver. Thus, failure to create those files is not critical.

    I have plenty other examples.

    Thus, we have two choices here:

    - The simple one: sysfs_create_blah() displays a warning when it fails
    and has no must_check

    - The one that adds code everywhere (the current one):
    sysfs_create_blah() returns an error, has much_check, and thus all
    callers like I described abvoe need to add code to test it and print a
    warning. Lots of added .text and .data for little benefit.


    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2007-07-21 02:35    [W:0.024 / U:2.576 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site