On Saturday, July 30, 2016 9:41:48 PM CEST Matthew White wrote: > Hello! > > I think that sometimes it could help to keep downloaded Metalink's files > which have a bad hash. > > The default wget behaviour is to delete such files. > > This patch provides a way to keep files which have a bad hash through the > option --keep-badhash. It appends the suffix .badhash to the file name, > except without overwriting existing files. In the latter case, an unique > suffix is appended after .badhash. > > I made this patch working on the following branch: > master (latest 20cac2c5ab3d63aacfba35fb10878a2d490e2377) > git://git.savannah.gnu.org/wget.git > > What do you think?
Hi Matthew, good work ! While your FSF assignment is underway (PM), we can continue polishing. Didn't test your code yet, but from what I see, there are just these peanuts: 1. + bhash = malloc (strlen (name) + strlen (".badhash") + 1); + strcat (strcpy (bhash, name), ".badhash"); Please consider using concat_strings() from util.h. And please leave an empty line between var declaration and code - just for readability. 2. Please add 'link' and 'unlink' to bootstrap.conf. Gnulib will emulate these on platforms where they are not available (or have a slightly different behavior). I guess we simply forgot 'unlink' when we switched to gnulib. 3. The (GNU) commit messages should ideally look like: one line of short description <empty line> file changes <empty line> long description For example: Add new option --keep-badhash * src/init.c: Add keepbadhash * src/main.c: Add keep-badhash * src/options.h: Add keep_badhash * doc/wget.texi: Add docs for --keep-badhash * src/metalink.h: Add prototypes badhash_suffix(), badhash_or_remove() * src/metalink.c: New functions badhash_suffix(), badhash_or_remove(). (retrieve_from_metalink): Call append .badhash() With --keep-badhash, append .badhash to Metalink's files with checksum mismatch, except without overwriting existing files. Without --keep-badhash, remove downloaded files with checksum mismatch (this conforms to the old behaviour). [This also applies to to your other patches] 4. Please not only write 'keepbadhash', but also what you did (e.g. remove, rename, add, ...), see my example above. Those ChangeLog entries should allow finding changes / faults / regressions etc. even when a versioning system is not at hand, e.g. when unpacking the sources from a tarball. (Not debatable that consulting commit messages directly is much more powerful.) 5. You write "except without overwriting existing files", maybe you should mention appending a counter instead of overwriting existent files !? Regards, Tim
signature.asc
Description: This is a digitally signed message part.