On Feb 8, 2008 9:01 AM, Jim Meyering <[EMAIL PROTECTED]> wrote:
> Matt Davis <[EMAIL PROTECTED]> wrote:
>
> > The following 3 patches implement history/undo/redo behavior for parted.
> >
> > This change captures potential disk modifications based on what commands a
> > user
> > issues. The list of disk modifications can be displayed/queried, undone, or
> > redone. Eventually the changes can be commited to disk. This prevents the
> > user
> > from performing unwanted modifications to disk before they are ready.
> > Also, the
> > user can view the disk as if the modifications were taking place, even
> > before
> > the "save" their changes.
> ...
> > diff --git a/libparted/history.c b/libparted/history.c
> ...
> > +char PedHistStr[32];
> ...
> > +PedHistManager PedHistMgr = {0};
> ...
> > +void
> > +ped_history_add (const char *name)
> ...
> > +void
> > +ped_history_clear (void)
>
> Hi Matt,
>
> I see at least two global variables there.
> The existing ones cause enough trouble, (and this is library code),
> so please don't add new global variables.
>
> As I said before, you will want to adjust each of your history-manipulation
> functions to take a pointer, probably *PedHistManager:
>
> void
> ped_history_add (PedHistManager *h, const char *name)
>
> which brings up another point:
> That function can fail (it calls malloc), hence it must not be void.
>
> Please look carefully at any new function that is "void".
> Each should probably should have a return type by which
> to indicate success or failure.
>
> Looking at ped_history_add, I see this:
>
> + addme->name = ped_malloc (name_length);
> + strncpy (addme->name, name, name_length);
>
> if ped_malloc fails, it returns NULL.
> If unchecked, strncpy will dereference that NULL,
> with the likely result of a segfault.
>
> Similarly for this:
>
> + addme = ped_history_alloc_obj ();
> + addme->id = PedHistMgr.id++;
>
> Please audit your patch for any similar problems.Here is my most recent implementation. If this is not cool please let me know. This is only implemented for the Linux architecture. Adding the initialization for the PedHistoryManager for other architectures should be similar. But I have not added them at this time, as I am waiting to make sure this patchset is appropriate :-) Thanks! -Matt
0001-Enables-undo-redo-of-disc-modifications-to-parted-co.patch
Description: Binary data
0002-Adds-the-history-undo-redo-functionality-to-libparte.patch
Description: Binary data
0003-Adds-test-script-for-the-history-undo-redo-parted-fu.patch
Description: Binary data
_______________________________________________ parted-devel mailing list [email protected] http://lists.alioth.debian.org/mailman/listinfo/parted-devel
