Re: [Bug-ddrescue] patch to fill error areas with specific data (--fill, --fill-from)

[Christian Franke]

Antonio Diaz Diaz wrote:
> Christian Franke wrote:
> > the attached patch adds a feature I recently missed when I recovered 
> > a damaged DVD with ddrescue:
> >
> > -f, --fill="STRING" -- Fills error areas with "STRING" instead of nulls.
> >
> > -F, --fill-from=FILE -- Same, but data is read from FILE.
>
> Seems an interesting feature, but I think it can be implemented more 
> efficiently and reliably as a separate pass. What do you all think 
> about this?

Yes, using an extra (e.g. Logbook::fill_errors()) method to do this in a 
separate pass within do_rescue() is an alternative.

But then already detected error areas would not be filled if ddrescue is 
interrupted with ^C.
Therefore, I would prefer the on-the-fly method used in the patch.


Meantime, I found a bug in the patch:
If the FILE is empty, ddrescue would hang because (!fill_data == 
(fill_size == 0)) does not hold.
And a delete is missing.

Fix:

    if (fill_size < 0) show_error( "read error on fill data file", errno );
+    if (fill_size == 0) show_error( "fill data file is empty" );
    close (fd);
-    if (fill_size < 0) return 1;
+    if (fill_size <= 0) { delete [] fill_data; return 1; }


Christian