[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH parted 1/5] dasd: Cleanup anchor handling
From: |
Hans de Goede |
Subject: |
Re: [PATCH parted 1/5] dasd: Cleanup anchor handling |
Date: |
Wed, 04 Nov 2009 11:00:24 +0100 |
User-agent: |
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.4pre) Gecko/20090922 Fedora/3.0-3.9.b4.fc12 Thunderbird/3.0b4 |
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