| Message ID | 1372593106-6593-2-git-send-email-fdmanana@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Sun, Jun 30, 2013 at 12:51:44PM +0100, Filipe David Borba Manana wrote: > Assert that the write of the device tree root succeeds. > This verification is currently done for all other tree > roots, however it was missing for the device tree root. > > Would this tree root write fail, but all others succeed, > it would lead to a corrupted/incomplete btrfs filesystem. > > Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com> > --- > utils.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/utils.c b/utils.c > index 7b4cd74..43d93f1 100644 > --- a/utils.c > +++ b/utils.c > @@ -381,6 +381,7 @@ int make_btrfs(int fd, const char *device, const char *label, > btrfs_set_header_nritems(buf, nritems); > csum_tree_block_size(buf, BTRFS_CRC32_SIZE, 0); > ret = pwrite(fd, buf->data, leafsize, blocks[4]); > + BUG_ON(ret != leafsize); Please fix also the 2nd instance where the return value check is missing 344 btrfs_set_header_bytenr(buf, blocks[3]); 345 btrfs_set_header_owner(buf, BTRFS_CHUNK_TREE_OBJECTID); 346 btrfs_set_header_nritems(buf, nritems); 347 csum_tree_block_size(buf, BTRFS_CRC32_SIZE, 0); 348 ret = pwrite(fd, buf->data, leafsize, blocks[3]); 349 350 /* create the device tree */ 351 memset(buf->data+sizeof(struct btrfs_header), 0, 352 leafsize-sizeof(struct btrfs_header)); > > /* create the FS root */ > memset(buf->data+sizeof(struct btrfs_header), 0, Also, replacing the BUG_ON with something else would be nice of course. thanks, david -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 3, 2013 at 6:09 PM, David Sterba <dsterba@suse.cz> wrote: > On Sun, Jun 30, 2013 at 12:51:44PM +0100, Filipe David Borba Manana wrote: >> Assert that the write of the device tree root succeeds. >> This verification is currently done for all other tree >> roots, however it was missing for the device tree root. >> >> Would this tree root write fail, but all others succeed, >> it would lead to a corrupted/incomplete btrfs filesystem. >> >> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com> >> --- >> utils.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/utils.c b/utils.c >> index 7b4cd74..43d93f1 100644 >> --- a/utils.c >> +++ b/utils.c >> @@ -381,6 +381,7 @@ int make_btrfs(int fd, const char *device, const char *label, >> btrfs_set_header_nritems(buf, nritems); >> csum_tree_block_size(buf, BTRFS_CRC32_SIZE, 0); >> ret = pwrite(fd, buf->data, leafsize, blocks[4]); >> + BUG_ON(ret != leafsize); > > Please fix also the 2nd instance where the return value check is > missing Good catch, thanks. Adding it now. > > 344 btrfs_set_header_bytenr(buf, blocks[3]); > 345 btrfs_set_header_owner(buf, BTRFS_CHUNK_TREE_OBJECTID); > 346 btrfs_set_header_nritems(buf, nritems); > 347 csum_tree_block_size(buf, BTRFS_CRC32_SIZE, 0); > 348 ret = pwrite(fd, buf->data, leafsize, blocks[3]); > 349 > 350 /* create the device tree */ > 351 memset(buf->data+sizeof(struct btrfs_header), 0, > 352 leafsize-sizeof(struct btrfs_header)); > >> >> /* create the FS root */ >> memset(buf->data+sizeof(struct btrfs_header), 0, > > Also, replacing the BUG_ON with something else would be nice of course. If you don't mind, I would prefer to do that as a different patch, since the BUG_ON() use is what is currently done everywhere in this function at least. thanks > > thanks, > david -- Filipe David Manana, "Reasonable men adapt themselves to the world. Unreasonable men adapt the world to themselves. That's why all progress depends on unreasonable men." -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 03, 2013 at 06:25:59PM +0100, Filipe David Manana wrote: > > Also, replacing the BUG_ON with something else would be nice of course. > > If you don't mind, I would prefer to do that as a different patch, > since the BUG_ON() use is what is currently done everywhere in this > function at least. Yes, separate patch. thanks, david -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/utils.c b/utils.c index 7b4cd74..43d93f1 100644 --- a/utils.c +++ b/utils.c @@ -381,6 +381,7 @@ int make_btrfs(int fd, const char *device, const char *label, btrfs_set_header_nritems(buf, nritems); csum_tree_block_size(buf, BTRFS_CRC32_SIZE, 0); ret = pwrite(fd, buf->data, leafsize, blocks[4]); + BUG_ON(ret != leafsize); /* create the FS root */ memset(buf->data+sizeof(struct btrfs_header), 0,
Assert that the write of the device tree root succeeds. This verification is currently done for all other tree roots, however it was missing for the device tree root. Would this tree root write fail, but all others succeed, it would lead to a corrupted/incomplete btrfs filesystem. Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com> --- utils.c | 1 + 1 file changed, 1 insertion(+)