On Wed, Aug 24, 2016 at 4:53 PM, Dmitry Bogatov wrote: > dget -x http://mentors.debian.net/debian/pool/main/m/mmh/mmh_0.3-1.dsc
I don't intend to sponsor this but here is a review: Please include the upstream tarball signature alongside the upstream tarball. uscan doesn't yet put it in the right place, but it should be named mmh_0.3.orig.tar.gz.asc and dpkg-buildpackage will include it into the .dsc file. These should get fixed before this package is suitable for upload in my opinion: sbr/dtimep.c is a generated file (using flex I think), please remove it in `debian/rules build` before ./configure is run so that we know we can build it from source. You'll probably need to also remove it in `debian/rules clean`. I found this using licensecheck. These would be nice to fix in my opinion: I don't think this is needed in the patches, just the headers should be enough: This patch header follows DEP-3: http://dep.debian.net/deps/dep3/ Personally, I wrap debian/watch after the opts line with a continuation character ("\"). Upstream should probably bump their copyright years, since they worked on it in 2016. Please add some upstream metadata: https://wiki.debian.org/UpstreamMetadata Automatic checks: build: gcc -c -DHAVE_CONFIG_H -D_GNU_SOURCE -I.. -I. -I.. -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fdebug-prefix-map=/build/mmh-0.3=. -fPIE -fstack-protector-strong -Wformat -Werror=format-security path.c path.c: In function 'expanddir': path.c:247:3: warning: ignoring return value of 'getcwd', declared with attribute warn_unused_result [-Wunused-result] getcwd(buf, sizeof buf); ^~~~~~~~~~~~~~~~~~~~~~~ gcc -c -DHAVE_CONFIG_H -D_GNU_SOURCE -I.. -I. -I.. -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fdebug-prefix-map=/build/mmh-0.3=. -fPIE -fstack-protector-strong -Wformat -Werror=format-security utils.c utils.c: In function 'pwd': utils.c:94:4: warning: ignoring return value of 'chdir', declared with attribute warn_unused_result [-Wunused-result] chdir(curwd); ^~~~~~~~~~~~ whatnowproc.c: In function 'what_now': whatnowproc.c:96:3: warning: ignoring return value of 'chdir', declared with attribute warn_unused_result [-Wunused-result] chdir(cwd); ^~~~~~~~~~ gcc -c -DHAVE_CONFIG_H -D_GNU_SOURCE -I. -I.. -I.. -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fdebug-prefix-map=/build/mmh-0.3=. -fPIE -fstack-protector-strong -Wformat -Werror=format-security mhshowsbr.c mhshowsbr.c: In function 'show_content_aux2': mhshowsbr.c:478:4: warning: ignoring return value of 'chdir', declared with attribute warn_unused_result [-Wunused-result] chdir(cracked); ^~~~~~~~~~~~~~ gcc -c -DHAVE_CONFIG_H -D_GNU_SOURCE -I. -I.. -I.. -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fdebug-prefix-map=/build/mmh-0.3=. -fPIE -fstack-protector-strong -Wformat -Werror=format-security new.c new.c: In function 'check_folders': new.c:226:3: warning: ignoring return value of 'chdir', declared with attribute warn_unused_result [-Wunused-result] chdir(toabsdir("+")); ^~~~~~~~~~~~~~~~~~~~ gcc -c -DHAVE_CONFIG_H -D_GNU_SOURCE -I. -I.. -I.. -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fdebug-prefix-map=/build/mmh-0.3=. -fPIE -fstack-protector-strong -Wformat -Werror=format-security dropsbr.c dropsbr.c: In function 'mbox_open': dropsbr.c:75:3: warning: ignoring return value of 'chown', declared with attribute warn_unused_result [-Wunused-result] chown(file, uid, gid); ^~~~~~~~~~~~~~~~~~~~~ dropsbr.c: In function 'mbox_copy': dropsbr.c:171:4: warning: ignoring return value of 'write', declared with attribute warn_unused_result [-Wunused-result] write(to, ">", 1); ^~~~~~~~~~~~~~~~~ gcc -c -DHAVE_CONFIG_H -D_GNU_SOURCE -I. -I.. -I.. -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fdebug-prefix-map=/build/mmh-0.3=. -fPIE -fstack-protector-strong -Wformat -Werror=format-security refile.c refile.c: In function 'opnfolds': refile.c:246:3: warning: ignoring return value of 'chdir', declared with attribute warn_unused_result [-Wunused-result] chdir(toabsdir("+")); ^~~~~~~~~~~~~~~~~~~~ refile.c:261:3: warning: ignoring return value of 'chdir', declared with attribute warn_unused_result [-Wunused-result] chdir(maildir); ^~~~~~~~~~~~~~ gcc -c -DHAVE_CONFIG_H -D_GNU_SOURCE -I. -I.. -I.. -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fdebug-prefix-map=/build/mmh-0.3=. -fPIE -fstack-protector-strong -Wformat -Werror=format-security rmf.c rmf.c: In function 'rmf': rmf.c:199:2: warning: ignoring return value of 'chdir', declared with attribute warn_unused_result [-Wunused-result] chdir(".."); ^~~~~~~~~~~ gcc -c -DHAVE_CONFIG_H -D_GNU_SOURCE -I. -I.. -I.. -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fdebug-prefix-map=/build/mmh-0.3=. -fPIE -fstack-protector-strong -Wformat -Werror=format-security slocal.c slocal.c: In function 'main': slocal.c:300:3: warning: ignoring return value of 'chdir', declared with attribute warn_unused_result [-Wunused-result] chdir("/"); ^~~~~~~~~~ slocal.c:305:3: warning: ignoring return value of 'setgid', declared with attribute warn_unused_result [-Wunused-result] setgid(pw->pw_gid); ^~~~~~~~~~~~~~~~~~ slocal.c:307:3: warning: ignoring return value of 'setuid', declared with attribute warn_unused_result [-Wunused-result] setuid(pw->pw_uid); ^~~~~~~~~~~~~~~~~~ slocal.c: In function 'usr_pipe': slocal.c:1001:3: warning: ignoring return value of 'freopen', declared with attribute warn_unused_result [-Wunused-result] freopen("/dev/null", "w", stdout); slocal.c:1002:3: warning: ignoring return value of 'freopen', declared with attribute warn_unused_result [-Wunused-result] freopen("/dev/null", "w", stderr); gcc -c -DHAVE_CONFIG_H -D_GNU_SOURCE -I. -I.. -I.. -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fdebug-prefix-map=/build/mmh-0.3=. -fPIE -fstack-protector-strong -Wformat -Werror=format-security whatnow.c whatnow.c: In function 'main': whatnow.c:241:5: warning: ignoring return value of 'fgets', declared with attribute warn_unused_result [-Wunused-result] fgets(cwd, sizeof (cwd), f); ^~~~~~~~~~~~~~~~~~~~~~~~~~~ dh_strip objcopy: debian/mmh/usr/bin/mh/stjT0pTt: debuglink section already exists objcopy: debian/mmh/usr/bin/mh/stwuMjru: debuglink section already exists objcopy: debian/mmh/usr/bin/mh/st0iPH7u: debuglink section already exists objcopy: debian/mmh/usr/bin/mh/stRl4zD0: debuglink section already exists objcopy: debian/mmh/usr/bin/mh/stlkUwm4: debuglink section already exists objcopy: debian/mmh/usr/bin/mh/stQO7sm9: debuglink section already exists objcopy: debian/mmh/usr/bin/mh/stU2ClAc: debuglink section already exists check-all-the-things: $ cppcheck -j1 --quiet -f . [sbr/makedir.c:42]: (error) Uninitialized variable: path [uip/distsbr.c:45]: (error) Resource leak: ifp [uip/distsbr.c:50]: (error) Resource leak: ifp [uip/distsbr.c:50]: (error) Resource leak: ofp [uip/distsbr.c:148]: (error) Resource leak: ifp [uip/distsbr.c:154]: (error) Resource leak: ifp [uip/mhmisc.c:231]: (error) va_list 'arglist' was opened but not closed by va_end(). $ find -type f -iname '*.sh' -exec checkbashisms {} + script ./test/common.sh does not appear to have a #! interpreter line; you may get strange results possible bashism in ./config/version.sh line 36 ($HOST(TYPE|NAME)): if [ X"$HOSTNAME" != X -a X"$HOSTNAME" != Xunknown ]; then possible bashism in ./config/version.sh line 43 ($HOST(TYPE|NAME)): echo "char *version_str = \"mmh-$VERSION [compiled on $HOSTNAME at `date`]\";" $ env PERL5OPT=-m-lib=. cme check dpkg ... Warning in 'control source Vcs-Git' value 'https://anonscm.debian.org/cgit/users/kaction-guest/mmh.git': URL to debian system is not the recommended one (this can be fixed with 'cme fix' command) ... Warning in 'copyright Format' value 'http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/': Format uses insecure http protocol instead of https $ codespell --quiet-level=3 <lots> $ find -type f -iname '*.c' -exec complexity {} + <lots, including these> ==> *seriously consider rewriting the procedure*. $ fdupes -q -r . | grep -vE '/(\.(git|svn|bzr|hg|sgdrawer|pc)|_(darcs|FOSSIL_)|CVS)(/|$)' | cat -s ./man/next.man1 ./man/prev.man1 ./man/fprev.man1 ./man/unseen.man1 ./man/fnext.man1 ./etc/components ./etc/forwcomps $ grep -Er '/(home|srv|opt)(\W|$)' . <some output, possibly useful> $ flawfinder -Q -c . <lots> # If you contact the owners of these keys, please point out OpenPGP best practices: # https://help.riseup.net/en/security/message-security/openpgp/best-practices $ find -type f -iname '*.asc' -exec cat {} + | hot dearmor | hokey lint ... Self-sig hash algorithms: [SHA-1] ... binding sig hash algorithms: [SHA-1] ... cross-cert hash algorithms: [SHA-1] # check if these can be switched to https:// $ grep -rF http: . <lots> $ find -type f \( -iname '*.c' -o -iname '*.cc' -o -iname '*.cxx' -o -iname '*.cpp' -o -iname '*.h' -o -iname '*.hh' -o -iname '*.hxx' -o -iname '*.hpp' \) -exec include-what-you-use {} \; <lots> $ isutf8 ./ChangeLog ./ChangeLog: line 17480, char 1, byte offset 65: invalid UTF-8 code $ env PERL5OPT=-m-lib=. license-reconcile File mismatch: Filter Std found cs/FAQ which was not in the file mapping. This probably implies a bug in the filter. at /usr/share/perl5/Debian/LicenseReconcile/App.pm line 222, <GEN0> line 3. File mismatch: Filter Std found cs/COMPLETION-BASH which was not in the file mapping. This probably implies a bug in the filter. at /usr/share/perl5/Debian/LicenseReconcile/App.pm line 222, <GEN0> line 3. File mismatch: Filter Std found bian/copyright which was not in the file mapping. This probably implies a bug in the filter. at /usr/share/perl5/Debian/LicenseReconcile/App.pm line 222, <GEN0> line 3. File mismatch: Filter Std found p/flist.c which was not in the file mapping. This probably implies a bug in the filter. at /usr/share/perl5/Debian/LicenseReconcile/App.pm line 222, <GEN0> line 3. $ find -type f \( -iname '*.sh' -o -iname '*.bash' -o -iname '*.zsh' \) -exec shellcheck {} + <lots> $ find -type d \( -iname .bzr -o -iname .git -o -iname .hg -o -iname .svn -o -iname CVS -o -iname RCS -o -iname SCCS -o -iname _MTN -o -iname _darcs -o -iname .pc -o -iname .cabal-sandbox -o -iname .cdv -o -iname .metadata -o -iname CMakeFiles -o -iname _build -o -iname _sgbak -o -iname autom4te.cache -o -iname blib -o -iname cover_db -o -iname node_modules -o -iname '~.dep' -o -iname '~.dot' -o -iname '~.nib' -o -iname '~.plst' \) -prune -o -type f ! \( -iname '*.bak' -o -iname '*.swp' -o -iname '#.*' -o -iname '#*#' -o -iname 'core.*' -o -iname '*~' -o -iname '*.gif' -o -iname '*.jpg' -o -iname '*.jpeg' -o -iname '*.png' -o -iname '*.min.js' -o -iname '*.js.map' -o -iname '*.js.min' -o -iname '*.min.css' -o -iname '*.css.map' -o -iname '*.css.min' -o -iname '*.wav' \) -exec env PERL5OPT=-m-lib=. spellintian --picky {} + <lots> $ grep -riE 'fixme|todo|hack|xxx+|broken' . <lots> -- bye, pabs https://wiki.debian.org/PaulWise
