ahrens commented on this pull request.
> get_metaslab_refcount(vdev_t *vd)
{
int refcount = 0;
if (vd->vdev_top == vd && !vd->vdev_removing) {
- for (int m = 0; m < vd->vdev_ms_count; m++) {
+ for (unsigned int m = 0; m < vd->vdev_ms_count; m++) {
we should be consistent about `unsigned` vs `unsigned int`, at least within
this file.
> @@ -1111,7 +1116,7 @@ dump_history(spa_t *spa)
umem_free(buf, SPA_MAXBLOCKSIZE);
(void) printf("\nHistory:\n");
- for (int i = 0; i < num; i++) {
+ for (unsigned i = 0; i < num; i++) {
Do you plan to make this change in the rest of the ZFS codebase? I think it
would be quite extensive. Can you expand a bit on what the benefit is?
TBH, I'm OK with making the changes to ZDB, but it's going to be hard to keep
following these rules without compiler assistance.
> @@ -3436,9 +3447,10 @@ zdb_read_block(char *thing, spa_t *spa)
uint64_t offset = 0, size = 0, psize = 0, lsize = 0, blkptr_offset = 0;
zio_t *zio;
vdev_t *vd;
- abd_t *pabd;
+ abd_t *pabd;
indentation looks wrong here
> extern uint8_t dump_opt[256];
static char prefix[4] = "\t\t\t";
static void
-print_log_bp(const blkptr_t *bp, const char *prefix)
+print_log_bp(const blkptr_t *bp, const char *_prefix)
leading `_` makes this seem pretty special. how about changing the global to
`tab_prefix`?
> {
+ lr_create_t *lr = (lr_create_t*) arg;
cast is not necessary (same goes for the other places)
> - { zil_prt_rec_link, "TX_LINK " },
- { zil_prt_rec_rename, "TX_RENAME " },
- { zil_prt_rec_write, "TX_WRITE " },
- { zil_prt_rec_truncate, "TX_TRUNCATE " },
- { zil_prt_rec_setattr, "TX_SETATTR " },
- { zil_prt_rec_acl, "TX_ACL_V0 " },
- { zil_prt_rec_acl, "TX_ACL_ACL " },
- { zil_prt_rec_create, "TX_CREATE_ACL " },
- { zil_prt_rec_create, "TX_CREATE_ATTR " },
- { zil_prt_rec_create, "TX_CREATE_ACL_ATTR " },
- { zil_prt_rec_create, "TX_MKDIR_ACL " },
- { zil_prt_rec_create, "TX_MKDIR_ATTR " },
- { zil_prt_rec_create, "TX_MKDIR_ACL_ATTR " },
- { zil_prt_rec_write, "TX_WRITE2 " },
+ { NULL, "Total ", 0 },
+ { zil_prt_rec_create, "TX_CREATE ", 0 },
maybe changing this to the new-style `.zri_print=zil_prt_rec_create,
.zri_name="TX_CREATE"` would be better (assuming the compiler is OK with
leaving out `zri_count`)
> @@ -39,7 +39,7 @@ extern "C" {
* particular object, use FTAG (which is a string) for the holder_tag.
* Otherwise, use the object that holds the reference.
*/
-#define FTAG ((char *)__func__)
+#define FTAG ((char *)(uintptr_t)__func__)
why do we need to cast this to an integer first?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/354#pullrequestreview-39611376
------------------------------------------
openzfs-developer
Archives:
https://openzfs.topicbox-beta.com/groups/developer/discussions/T95949071eb220b14-M251556954ca06ed2080baf37
Powered by Topicbox: https://topicbox-beta.com
