On 3/19/15 10:07 AM, Brian Foster wrote:
> On Tue, Mar 17, 2015 at 03:33:05PM -0500, Eric Sandeen wrote:
>> This adds a new "crc" command to xfs_db for CRC-enabled filesystems.
...
if (argc) while ((c = getopt(argc, argv, "irv")) != EOF) {
>
> The 'if (argc)' thing strikes again. Does this address something I'm not
> aware of?
just a dumb/unfortunate cut & paste from elsewhere, I'll fix.
...
>> + /* Off by one.. */
>> + crc = cpu_to_be32(crc + 1);
>> + setbitval(iocur_top->data, sfl->offset, bit_length, &crc);
>
> Some comments on the above code would be nice to explain what it's
> calculating.
Yeah, wish I'd written them "yonks ago" ;) I'll reverse engineer my
patch & add comments.
>> + dbprintf(_("Verifying CRC:\n"));
>
> Does the "verify" aspect of this command do anything, or are we just
> printing the state that was determined from a previous cursor load? If
> the latter, I wonder if it's worth retaining that option (e.g., just
> print an inode to see the crc state)..?
If having a specific CRC handler makes sense (and, maybe it doesn't,
with the other patch that allows corrupted write), the symmetry seemed
nice.
Given the amount of code this adds, perhaps we should just drop it,
TBH, and use the "write -c" functionality I added, instead.
-Eric
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
