On Fri, Aug 21, 2009 at 05:15:34PM +0200, Jim Meyering wrote: > Petr Uzel wrote: > > * tests/t2300-dos-label-extended-bootcode.sh: New file. > > * tests/Makefile.am (TESTS): Add t2300-dos-label-extended-bootcode.sh. > > Hi Petr, > > This looks good. > A few suggestions below.
Hi Jim, Thanks for the comments. > > +require_512_byte_sector_size_ > > + > > +# Note: the bootcode size is 440B > > Rather than that comment, please just define a well-named variable, > > bootcode_size=440 > > And then use it in place of the two hard-coded constants below. Hmm, actually in the first version of the patch there was this variable, but I was convinced not to use it: http://www.mail-archive.com/[email protected]/msg02450.html > > + > > +test_expect_success \ > > + 'Install fake bootcode' \ > > + 'dd if=/dev/urandom of=$dev bs=1c seek=16384 count=400 \ > > Why do you use /dev/urandom? > That might not exist. How about just using e.g., if=Makefile ? if=Makefile does not work, but if=../Makefile works. BTW the same issue is in t0202-gpt-pmbr.sh > > Shouldn't the above count=400 be count=440 (or now, count=$bootcode_size)? Yep, looks like a typo. I'll post a patch to address these issues. -- Best regards / s pozdravem Petr Uzel, openSUSE Community Multiplier Team ----------------------------------------------------------------- SUSE LINUX, s.r.o. e-mail: [email protected] Lihovarská 1060/12 http://www.suse.cz 190 00 Prague 9, CR
pgpeaZmoORDZK.pgp
Description: PGP signature
_______________________________________________ parted-devel mailing list [email protected] http://lists.alioth.debian.org/mailman/listinfo/parted-devel
