linux-kernel - Re: [PATCHv2] drivers:ata: Remove unneeded fix me comment for the function,mv_print_info in sata

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>] [day] [month] [year] [list]

Message-ID: <20141226221227.GC10889@htj.dyndns.org>
Date:	Fri, 26 Dec 2014 17:12:27 -0500
From:	Tejun Heo <tj@...nel.org>
To:	Nicholas Krause <xerofoify@...il.com>
Cc:	linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCHv2] drivers:ata: Remove unneeded fix me comment for the
 function,mv_print_info in sata_mv.c

Hello, Nicholas.

On Fri, Dec 26, 2014 at 04:55:59PM -0500, Nicholas Krause wrote:
> Removes a unneed fix me comment for the function,mv_print_info in sata_mv.c.
> This function is correctly completed due to printing out needed info about 
> hardware to the kernel log buffer. The needed information that is being 
> printed successfully now to the callers of this function is the host 
> number of ports, the generation of hardware, the maximum q depth the
> hardware supports, if the hardware is either RAID or SCSI enabled and
> finally the IRQ mode enabled when called and the flag enabled with the
> respectful IRQ called. 

It's often more effective to discuss to conclusion on thread before
sending another patch.

When tracking down FIXME comments, it's often a good idea to look at
how the comment came to be by digging through the history.  git blame
on the file shows that the comment was added by 05b308e1df6d ("[PATCH]
libata: Marvell function headers") a long time ago.  Compared to then,
the function grew generation info printing, which could have been what
the original FIXME author had in mind or maybe not.  Regardless, given
that nobody has complained about lacking information over the years,
it can be argued that the FIXME is spurious.

So, I don't object to the patch itself.  These are just taking way too
much effort given that the actual gain is almost nil.  If you want to
continue to do this, please research about the history of FIXME and
construct and present a valid rationale for the change.  For this one,
listing what it prints out doesn't constitute rationale in itself.

Please do keep in mind these changes are ultimately frivolous.  If
they incur more cost than they're beneficial, they're pointless, so
please send properly formed and reasoned patches.  This probably is my
last response on the subject.

Thanks.

-- 
tejun
--
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/