Re: [PATCH] repack: rewrite the shell script in C.
Am 21.08.2013 10:49, schrieb Matthieu Moy:
Show 15 quoted lines
> Stefan Beller <stefanbeller@googlemail.com> writes:
>> + for_each_string_list_item(item, &names) {
>> + for (ext = 0; ext < 2; ext++) {
>> + char *fname, *fname_old;
>> + fname = mkpathdup("%s/%s%s", packdir, item->string, exts[ext]);
>> + if (!file_exists(fname)) {
>> + free(fname);
>> + continue;
>> + }
>> +
>> + fname_old = mkpath("%s/old-%s%s", packdir, item->string, exts[ext]);
>> + if (file_exists(fname_old))
>> + unlink(fname_old);
>
> Unchecked returned value.Good catch! The original was 'rm -f ... && mv ... || failed=t'
Show 12 quoted lines
>> + /* Now the ones with the same name are out of the way... */
>> + for_each_string_list_item(item, &names) {
>> + for (ext = 0; ext < 2; ext++) {
>> + char *fname, *fname_old;
>> + struct stat statbuffer;
>> + fname = mkpathdup("%s/pack-%s%s", packdir, item->string, exts[ext]);
>> + fname_old = mkpath("%s-%s%s", packtmp, item->string, exts[ext]);
>> + if (!stat(fname_old, &statbuffer)) {
>> + statbuffer.st_mode &= ~S_IWUSR | ~S_IWGRP | ~S_IWOTH;
>> + chmod(fname_old, statbuffer.st_mode);
>
> Unchecked return value.The original was an unchecked 'chmod a-w', so we don't care.
Of course, we could mimic the original better by issuing warnings.
Show 14 quoted lines
>
>> + /* Remove the "old-" files */
>> + for_each_string_list_item(item, &names) {
>> + char *fname;
>> + fname = mkpath("%s/old-pack-%s.idx", packdir, item->string);
>> + if (remove_path(fname))
>> + die_errno(_("removing '%s' failed"), fname);
>> +
>> + fname = mkpath("%s/old-pack-%s.pack", packdir, item->string);
>> + if (remove_path(fname))
>> + die_errno(_("removing '%s' failed"), fname);
>
> Does this have to be a fatal error? If I read correctly, it wasn't fatal
> in the shell version.Johannes Sixt <j6t@kdbg.org>