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

Bug#841536: hdparm/9.50+ds-1



Control: tags -1 moreinfo

On Fri, 21 Oct 2016 14:46:32 +0200 Alex Mestiashvili
<mailatgoogl@gmail.com> wrote:
> Package: sponsorship-requests
> Severity: wishlist
> X-Debbugs-Cc: mailatgoogl@gmail.com
> 
> Dear mentors,
> 
>  [...]
> 
> Best regards,
> Alex


Hi Alex,

Thanks for your contribution.

Have you informed upstream about the following compiler warning?
"""
hdparm.c: In function 'process_dev':
hdparm.c:2256:2: warning: this 'if' clause does not guard...
[-Wmisleading-indentation]
  if (!quiet)
  ^~
hdparm.c:2258:3: note: ...this statement, but the latter is misleadingly
indented as if it is guarded by the 'if'
   if (do_drive_cmd(fd, args, 0)) {
   ^~
"""

I checked it seems benign, so I am not blocking the upload on this.  The
code in question being:


"""
	if (security_freeze) {
		__u8 args[4] = {ATA_OP_SECURITY_FREEZE_LOCK,0,0,0};
	if (!quiet)
        ^^^^^^^^^^^ (should be 1 tab further in)
		printf(" issuing security freeze command\n");
                ^^^^^^^^^^^^^^ (should be 1 tab further in as well)
		if (do_drive_cmd(fd, args, 0)) {
			err = errno;
			perror(" HDIO_DRIVE_CMD[...] failed");
		}
	}

"""

Other minor nits:
 * Spelling error in hdparm binary "Removeable" -> "Removable"
   - NB: Upstream is inconsistent with the spelling, but seems to prefer
     the "Removable" variant.
 * Upstream's build system hardcodes -j2 on "make all" and the packaging
   uses that without any respect to DEB_BUILD_OPTIONS
   - Given the small size and low parallel limit, I don't think it is a
     huge issue (but it would have been for other packages).
 * The debian/hdparm.preinst file appears to be redundant (its replaced
   by the debian/hdparm.maintscript)

Thanks,
~Niels

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: