Hi Niels, thank you for the comments and upload. I've prepared an updated package addressing the issues and uploaded it to mentors:
https://mentors.debian.net/debian/pool/main/h/hdparm/hdparm_9.50+ds-2.dsc On Fri, Oct 21, 2016 at 7:07 PM, Niels Thykier <ni...@thykier.net> wrote: > Control: tags -1 moreinfo > > On Fri, 21 Oct 2016 14:46:32 +0200 Alex Mestiashvili > <mailatgo...@gmail.com> wrote: > > Package: sponsorship-requests > > Severity: wishlist > > X-Debbugs-Cc: mailatgo...@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"); > } > } > > """ > The wrong indentation comes from the quiet_security_freeze.patch fixing the bug #512175 The indentation is fixed in the latest package. > > Other minor nits: > * Spelling error in hdparm binary "Removeable" -> "Removable" > - NB: Upstream is inconsistent with the spelling, but seems to prefer > the "Removable" variant. > I know about this typo, but didn't want to fix it because the fix would change the output of the program and it might impact scripts trying to identify that flag. I'll forward this and other typos to upstream. > * 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). > Fixed with the makefile.patch > * The debian/hdparm.preinst file appears to be redundant (its replaced > by the debian/hdparm.maintscript) > Thanks, fixed too.
