Re: [PATCH 1/9] parted: fix EOF and ctrl-c handling

[Brian C. Lane]

Sorry it has taken so long to look at these. Here's my comments on each.
Hopefully Jim will have time to look at them as well.

In general you need to watch for mixing tabs and spaces. If modifying
code that's already using tabs, match it. New code should use spaces
though, and in some places you dropped in a tab in the middle of a block
of spaces.

Commit messages need to follow the normal parted form. I'd also like to
see tests as separate commits.

1/9 - Ack.
2/9 - Ack.
3/9 - Ack. 0 sized data area just doesn't make sense anyway.
4/9 - Nak. I have a more general patch for this -- not everything
           inserts DMRAID in there.
5/9 - Shouldn't it use disk->dev->sector_size instead of hard-coded
      512/1024? The logic seems a bit messy, using an if block instead
      of the == 1 ? 512 : 1024 may clean it up a little. If it is
      already set to 512 (or really ->sector_size) we don't need to walk
      the other partitions. One place where a tab snuck in. Looks pretty
      ok other than that.
6/9 - Nak. I'm not sure if this is the right way to handle things.
7/9 - Nak. I really don't like the giant pile of changes for a problem
      that is only in the gpt code.
8/9 - Nak. It looks to me like removing some of these checks is going to
      allow user (ui or library) checks that should fail on new data to
      pass. I'm ok with letting us run with these errors so we can fix
      them, but you shouldn't be able to enter bad numbers after the
      warnings.
9/9 - Nak. I need more details here. The reasoning in the original
      commit seems sound to me, and I'd be more worried about busy
      devices than the filesystem not matching (I'm not sure we should
      even be in the fs detection business anymore).

-- 
Brian C. Lane | Anaconda Team | IRC: bcl #anaconda | Port Orchard, WA (PST8PDT)

pgpmSdKZkKk9y.pgp
Description: PGP signature