Date: Mon, 12 Mar 2012 14:10:11 +0100 From: Ivan Voras <ivoras@freebsd.org> To: freebsd-current@freebsd.org Cc: freebsd-geom@freebsd.org Subject: Re: [RFC, RFT] LDM support (aka Windows Dynamic Volumes) Message-ID: <jjksjk$bfv$1@dough.gmane.org> In-Reply-To: <4F5C71DC.3010203@yandex.ru> References: <4F5C71DC.3010203@yandex.ru>
index | next in thread | previous in thread | raw e-mail
[-- Attachment #1 --] On 11/03/2012 10:35, Andrey V. Elsukov wrote: > Hi, All > > i wrote GEOM_PART_LDM class. It provides basic support of Logical Disk Manager > partitioning scheme [1]. Since LDM metadata is not documented i used several > articles found in the web and linux implementation as reference [2]. Seems ok, but there are a lot of "magic numbers" sprinkled around, which could possibly be better expressed as named constants / macros. E.g. in ldm_privhdr_parse(), ldm_tochdr_check(), as the array size in line 434, in the switch block in lines 635, etc. I don't think it's a big problem, though. You could use SLIST_FOREACH_SAFE in ldm_vmdb_free(), but that's also not a problem. You have a lot of deeply nested loops in ldm_vmdb_parse() and others, which may be "flattened" by removing the inner loops into separate functions (so you don't run into the 80 column limit so often), but again, not a problem. Looks ok. [-- Attachment #2 --] -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.18 (FreeBSD) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk9d9bMACgkQldnAQVacBcjJqwCfU+OnCw9xR5mmvUehDA8fg7pR MRcAoLa587XhZ4GMboBrwiFVjCpct9qT =TnkD -----END PGP SIGNATURE-----help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?jjksjk$bfv$1>