Hi,
On 11/04/2009 10:32 AM, Jim Meyering wrote:
Hans de Goede wrote:
The current dasd label code keeps an fdasd anchor struct in the
DasdDiskSpecific struct, and fills this during dasd_read. However this
anchor does not get updated with any future mods, until dasd_write,
at which points it gets completely re-initialized.
Thanks for these patches.
My first pass at reviewing them spotted only a few nits:
- it requires a test (outline is fine, as before)
Not sure how helpful that would be as running such a test requires doing
so on an s390 virtual machine.
Anyways outline:
-before do ped_disk_fresh_new(dasd_device, ped_disk_type_get("dasd") would
always
fail, as the ioctl in dasd_alloc would fail with a BADFD errno
-after this works
-before doing ped_disk_duplicate() on a PedDisk with a dasd DiskType, would
fail triggering an assert (no partition_duplicate implementation), now it
works.
- any fix like that deserves an entry in NEWS;
Please add an entry for your new alignment-related functions, while
you're at it.
Will do.
- the new index, "int i" (in 2/5) should be declared "unsigned int i".
Also, please move the declaration so it is right before the first use:
i.e., declare it on the line right before the for-loop.
In general, please move declarations you add or touch "down"
to be as close to first use as possible.
I duplicated the new way the loop is done from the loop in dasd_write,
and i is a signed int there (in dasd_write) too. The same for the declaration,
also note that declaring variables after statements is not allowed by ANSI-C
(89),
it is by ISO C99.
In general I'm a favor of declaring all the variables at the top coding style
(and this keeps the code compilable by any ANSI-C compiler), but if the coding
style for parted is to declare variables close to their first use, I'll adapt.
Regards,
Hans
_______________________________________________
bug-parted mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/bug-parted
