On Wed, 12 Sep 2018 15:36:32 +0800 Michael Mikonos <[email protected]> wrote:
> On Wed, Sep 12, 2018 at 12:13:35AM -0700, Ori Bernstein wrote: > > On Tue, 11 Sep 2018 23:29:53 -0700, Ori Bernstein <[email protected]> > > wrote: > > > > > static ssize_t > > > @@ -362,8 +377,9 @@ qc2_close(void *p) > > > struct qcdisk *disk; > > > > > > disk = p; > > > - pwrite(disk->fd, disk->l1, disk->l1sz, disk->l1off); > > > - close(disk->fd); > > > + if (disk->base) > > > + qc2_close(disk->base); > > > + free(disk->l1); > > > free(disk); > > > } > > > > > > > Er, of course, the close should still exist here. > > I was curious about qc2_close() calling itself for disk->base. We open it recursively for multi-disk snapshots, where the disk specifies the base to write deltas off of. We should close it the same way. > Also, is it worth setting disk->fd = -1 after close(), and > adding a check to prevent closing it again? The data is in a malloc'ed structure we're freeing on the next line. I'm not sure that there's much point in setting the fd to -1 and checking it: if we try closing the same fd again, we've got bigger problems. -- Ori Bernstein <[email protected]>
