Bug#287170: vorbiscomment truncates weird input files

[Bastian Kleineidam]

Hi,

the reason for the overwrite is the close_files() function in
vcomment.c. It always renames the temporary output file to the given
output file, regardless of previous failures.

I added an error flag and if set only remove the temporary filename.
This fixed the error for me.
Test was exactly like in the report (with a test.1 manpage file):
$ printf 'ARTIST=Carlos Santana and Mahavishnu John McLaughlin' | \
  vorbiscomment -w test.1

Regards,
  Bastian
-- 
  ,''`.                  Bastian Kleineidam
 : :' :                    GnuPG Schlüssel
 `. `'    gpg --keyserver wwwkeys.pgp.net --recv-keys 32EC6F3E
   `-


--- vorbis-tools-1.0.1/vorbiscomment/vcomment.c 2005-11-25 19:09:06.878957929 
+0100
+++ vorbis-tools-1.0.1_/vorbiscomment/vcomment.c        2005-11-25 
19:08:42.298707644 +0100
@@ -58,7 +58,7 @@
 void free_param(param_t *param);
 void parse_options(int argc, char *argv[], param_t *param);
 void open_files(param_t *p);
-void close_files(param_t *p);
+void close_files(param_t *p, int err);
 
 
 /**********
@@ -102,7 +102,7 @@
                {
                        fprintf(stderr, _("Failed to open file as vorbis: 
%s\n"), 
                                        vcedit_error(state));
-            close_files(param);
+            close_files(param, 1);
             free_param(param);
             vcedit_clear(state);
                        return 1;
@@ -115,7 +115,7 @@
                /* done */
                vcedit_clear(state);
 
-               close_files(param);
+               close_files(param, 0);
         free_param(param);
                return 0;               
        }
@@ -128,7 +128,7 @@
                {
                        fprintf(stderr, _("Failed to open file as vorbis: 
%s\n"), 
                                        vcedit_error(state));
-            close_files(param);
+            close_files(param, 1);
             free_param(param);
             vcedit_clear(state);
                        return 1;
@@ -169,7 +169,7 @@
                {
                        fprintf(stderr, _("Failed to write comments to output 
file: %s\n"), 
                                        vcedit_error(state));
-            close_files(param);
+            close_files(param, 1);
             free_param(param);
             vcedit_clear(state);
                        return 1;
@@ -178,7 +178,7 @@
                /* done */
                vcedit_clear(state);
                
-               close_files(param);
+               close_files(param, 0);
         free_param(param);
                return 0;
        }
@@ -453,7 +453,6 @@
 void open_files(param_t *p)
 {
        /* for all modes, open the input file */
-
        if (strncmp(p->infilename,"-",2) == 0) {
                p->in = stdin;
        } else {
@@ -529,7 +528,7 @@
 
 ***********/
 
-void close_files(param_t *p)
+void close_files(param_t *p, int err)
 {
        if (p->in != NULL && p->in != stdin) fclose(p->in);
        if (p->out != NULL && p->out != stdout) fclose(p->out);
@@ -539,13 +538,21 @@
         /* Some platforms fail to rename a file if the new name already exists,
          * so we need to remove, then rename. How stupid.
          */
-               if(rename(p->outfilename, p->infilename)) {
-            if(remove(p->infilename))
-                fprintf(stderr, _("Error removing old file %s\n"), 
p->infilename);
-            else if(rename(p->outfilename, p->infilename)) 
-                fprintf(stderr, _("Error renaming %s to %s\n"), 
p->outfilename, 
-                        p->infilename);
+            if (err != 0)
+            {
+                if(remove(p->outfilename)) {
+                    fprintf(stderr, _("Error removing temp file %s\n"), 
p->outfilename);
+                }
+            }
+            else {
+                if(rename(p->outfilename, p->infilename)) {
+                    if(remove(p->infilename))
+                        fprintf(stderr, _("Error removing old file %s\n"), 
p->infilename);
+                    else if(rename(p->outfilename, p->infilename))
+                        fprintf(stderr, _("Error renaming %s to %s\n"), 
p->outfilename,
+                                p->infilename);
+                }
+            }
         }
-    }
 }
 

signature.asc
Description: Digital signature