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

Re: Bug#793651: RFS: hdump/2.3-1 [ITP] -- Hexadecimal and ASCII dumper for binary files



-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi Jakub,

thank you very much for your review. 
I will fix my mistakes in packaging and send to upstream your observations.

regards,
[]'s
kretcheu
:x


On 26-07-2015 09:04, Jakub Wilk wrote:
> I don't intend to upload this package, but here's my review:
> 
> * Paulo <kretcheu@gmail.com>, 2015-07-25, 21:16:
>> http://mentors.debian.net/debian/pool/main/h/hdump/hdump_2.3-1.dsc
> 
> The package description reads:
> 
>> Description: Hexadecimal and ASCII dumper for binary files
> 
> The synopsis is not a sentence, so no need to start it with a capital letter. (Developer's Reference §6.2.2)
> 
> 
>> Fast and simple hexadecimal/ASCII dumper for binary files,
> 
> OTOH, the long description should consist of full sentences. (Developer's Reference §6.2.3)
> 
>> written in ANSI C.
> 
> That's not something end users care, so I don't think it should belong in the package description.
> 
>>   - Multi-platform (tested on GNU/Linux and Windows).
> 
> Again, not relevant for end users.
> 
>>   - Specify the initial byte (-b). Supports hex notation.
> 
> Huh, what? There's no such option. (Not that I know what it would be useful for...)
> 
>>   - Define numbers of bytes (-n). Multiple of the number of columns.
> 
> I don't understand what the second sentence is supposed to mean.
> 
> The patch header reads:
> 
>> Description: fix some issues in upstream Makefile to hardening.
> 
> That's not very informative. I would have no idea what this patch does without reading the actual diff.
> 
> Also, please forward the patch upstream.
> 
> The Makefile passes both -W and -Wextra to gcc, even though -W is just an obsolete alias for -W.
> 
> I think fsprintf might be a typo.
> 
> debian/manpage/genallman.sh ignores all errors. (Although you don't use it debian/rules, so meh...)
> 
> The manpage, like the package description, says something about ANSI C, the mysterious -b option, and "multiple of the number of columns".
> 
> man-pages(7) strongly discourages AUTHORS sections, and so do I.
> I find the "for the Debian project (but may be used by others)" part particularly silly.
> 
> Upstream changelog reads:
> 
>> - Fixed compilation failure due misuse of fsprintf() function.
> 
> Lintian says:
> X: hdump: binary-file-built-without-LFS-support usr/bin/hdump
> 
> And indeed, the program doesn't support files bigger an 2GB on 32-bit architectures:
> 
> $ truncate -s 3G foo
> $ hdump foo
> file not found or not readable
> 
> The program ignores read and write errors:
> 
> $ hdump README.md > /dev/full
> $ echo $?
> 0
> 
> $ hdump /proc/self/mem
> $ echo $?
> 0
> 
> If you provide an option, but not the path, it treats the last argument as path:
> 
> $ hdump -c 0
> file not found or not readable
> 
> You can't dump non-seekable files:
> 
> $ echo foo | hdump /dev/stdin
> unable to seek through file
> 
> 
> [ This review was written for the debian-mentors mailing list (but may be read by others). ;-) ]
> 
> -- 
> Jakub Wilk
> 
> 
> -- 
> To UNSUBSCRIBE, email to debian-mentors-REQUEST@lists.debian.org
> with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
> Archive: [🔎] 20150726120452.GA9148@jwilk.net">https://lists.debian.org/[🔎] 20150726120452.GA9148@jwilk.net
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJVtPQpAAoJEDnxel9a775zD9QP/iQ9NekAV2TTKXiIktsgJpCd
XY5ubwPYSi5/T+b6rzDZBBj5u3ud5CUfyhdGQ4Mn20hP8g3+qWkLlFl7geqjx15G
EbN82KUTmp8YFDjAbWOpB1VFS2yCDPUb83moey7s8JFUXCGJbu4SrSN+RKk/gECO
fpMzSns19JsmBTk+24IyT1Re8SWuumivdUs/umsA0z094kTcKoLZVTlXhdifcKn9
/gGeTL6a11z6qx5khuF2oAxqNEDpWEyzT0I1RST9vZFvAoz2nzV0bcy3B8e+Wbxt
FkyDkCcWiST++zcvzQ6ba52EKiPqen6/giCgj9oXZCefbVqGKyXEoOi9MR7QRS0J
t5VatiDO7Zwv9WWVKgcT3GbuhjV7OLp1Hr8fFeu6Sz5p5619puqqKIn6T2yCqO27
B517xchowKqkIGvI3qPQWb2lUUPCUVZ5ymp/soh/RUUlIa65qTWEENOUQb4RiYv6
ag84+9ofoX63vWs2YwZ/+jF9GkyX8Z0Sgiwo/izK8fDrC20yGagUJLIw1i0gQTCx
RzUeH8r/MK6mzmM+RwfjOwhwFG4uwao86LHzz6ZaUzDpEVr/qwyTX/htfzBqnqW+
BkLN+bsMwrrfvXkcJQvLFC/GQ/mVUIJP2IdA7VSrWGdNmIgWKqMrhSwN7n42PtQW
uGcuX4u52NxTZmJJxEgD
=ChpH
-----END PGP SIGNATURE-----


Reply to: