On Tue, Jun 09, 2009 at 11:06:28AM +0200, Jim Meyering wrote: > Joel Granados wrote: > > > On Tue, Jun 09, 2009 at 10:10:16AM +0200, Joel Granados wrote: > >> On Fri, Jun 05, 2009 at 03:30:52PM +0200, Jim Meyering wrote: > >> > Joel Granados Moreno wrote: > >> > > From: Petr Uzel <[email protected]> > >> > > > >> > > * libparted/labels/dos.c (write_ext_table): Do not discard > >> > > bootcode from extended partition on msdos label when some of > >> > > the logical partitions are changed. > >> > > > >> > > Signed-off-by: Petr Uzel <[email protected]> > >> > > --- > >> > > libparted/labels/dos.c | 6 ++++-- > >> > > 1 files changed, 4 insertions(+), 2 deletions(-) > >> > > > >> > > diff --git a/libparted/labels/dos.c b/libparted/labels/dos.c > >> > > index f219e7d..4e308fe 100644 > >> > > --- a/libparted/labels/dos.c > >> > > +++ b/libparted/labels/dos.c > >> > > @@ -1060,7 +1060,8 @@ write_ext_table (const PedDisk* disk, > >> > > > >> > > lba_offset = ped_disk_extended_partition (disk)->geom.start; > >> > > > >> > > - memset (&table, 0, sizeof (DosRawTable)); > >> > > + ped_device_read (disk->dev, &table, sector, 1); > >> > > + memset (&(table.partitions), 0, 4 * sizeof(DosRawPartition)); > >> > > table.magic = PED_CPU_TO_LE16 (MSDOS_MAGIC); > >> > > > >> > > if (!fill_raw_part (&table.partitions[0], logical, sector)) > >> > > @@ -1094,7 +1095,8 @@ write_empty_table (const PedDisk* disk, > >> > > PedSector sector) > >> > > > >> > > PED_ASSERT (disk != NULL, return 0); > >> > > > >> > > - memset (&table, 0, sizeof (DosRawTable)); > >> > > + ped_device_read (disk->dev, &table, sector, 1); > >> > > + memset (&(table.partitions), 0, 4 * sizeof(DosRawPartition)); > >> > > table.magic = PED_CPU_TO_LE16 (MSDOS_MAGIC); > >> > > > >> > > return ped_device_write (disk->dev, (void*) &table, sector, 1); > >> > > >> > This has the same problem I mentioned for 1/14: > >> > it introduces new code that depends on fixed-size (512-byte) sectors. > >> > Thus it conflicts with changes on "next" that eliminated the 512-byte > >> > limitation. > >> > > >> > How about putting it on that branch instead? > >> > >> > >> Same answer as above.... > > > > Wait!!! How does this conflict with changes in the next branch? > > Take a look, or try to merge and then run the tests, > including with valgrind. > > There may not be an official version-control conflict, > but that change would certainly introduce a nasty bug. > > Quick answer: table is declared like this: > > DosRawTable table; > > yet your added ped_device_read call may write >512 bytes > (i.e., sector size) into that buffer, thus clobbering the stack.
Aha. I see, I thought you were only refering to merge conflicts. > > > Are you talking about your local changes? > > Of course not. Those would be private. Besides, I have none. :) > > > If you are pls commit so I can > > address this. Additionally, how does this depend on the sector size? > > Does the ped_device_read call change in your local next branch? And > > No, but it must work for sector sizes larger than 512 bytes. > > > "memset (&(table.partitions), 0, 4 * sizeof(DosRawPartition))" will be > > valid for all secto sizes. I mean, the RawPartition definition will not > > change even though the sector size does. I'll let Ptr address this issue... -- Joel Andres Granados Brno, Czech Republic, Red Hat. _______________________________________________ parted-devel mailing list [email protected] http://lists.alioth.debian.org/mailman/listinfo/parted-devel
