Skip to contentSkip to conversation

current e2fsprogs maint branch test results

DISCUSSION Eric Whitney 2 participants 3 messages Dec 17, 2013 – Dec 23, 2013

lore
Share & tools
lore
#0 lore raw

I've built the contents of the current e2fsprogs maint branch (4727c67dc2) and run make check on both a Pandaboard (ARM) and an x86-64 VM. In each case I used the following arguments to configure: --disable-uuidd --disable-libuuid --disable-libblkid.

Both the ARM and x86-64 runs produced warnings when compiling ea_refcount.c during make check. I posted a patch yesterday (e2fsck: fix printf conversion specs in ea_refcount.c) that clears those up for me on both platforms.

The ARM build and make check were otherwise clean.

The x86-64 build and make check contained one more problem - a warning while compiling debugfs.c:
../../debugfs/debugfs.c:2462:5: warning: too many arguments for format
[-Wformat-extra-args]

This code (commit fe56188b07) is part of that used to check superblock block numbers specified on the command line, and the error reporting has become a little fuzzy relative to what we had previously.

Before:

root@debug1:~# debugfs -s 327b /dev/vdc
debugfs 1.42.8 (20-Jun-2013)
debugfs: Bad superblock number - 327b

After:

root@debug1:~# debugfs -s 327b /dev/vdc
debugfs 1.42.8 (20-Jun-2013)
debugfs: Bad block number - 327b
debugfs: Invalid block number: 327b
debugfs: Operation not permitted 

Both strtoblk() and parseulonglong() (which it calls) output error messages for bad/invalid block numbers, which is redundant in this case. The last (erroneous) error message is output by the call to com_err() which also causes the warning noted above.

It seems to me that the call to com_err() ought to be deleted, and maybe the immediately preceding call to strtoblk() ought to be converted to a call to parseulonglong() to restore the original messaging. I'd like to post a patch, but there are a number of other calls to strtoblk() in debugfs now that will produce two messages on errors, and the intent isn't clear to me. Is this just an area that needed a little more polish?

Thanks,
Eric

#1 lore raw
Quoted 40 lines · Eric Whitney

I've built the contents of the current e2fsprogs maint branch (4727c67dc2) and run make check on both a Pandaboard (ARM) and an x86-64 VM. In each case I used the following arguments to configure: --disable-uuidd --disable-libuuid --disable-libblkid.

Both the ARM and x86-64 runs produced warnings when compiling ea_refcount.c during make check. I posted a patch yesterday (e2fsck: fix printf conversion specs in ea_refcount.c) that clears those up for me on both platforms.

The ARM build and make check were otherwise clean.

The x86-64 build and make check contained one more problem - a warning while compiling debugfs.c:
../../debugfs/debugfs.c:2462:5: warning: too many arguments for format
[-Wformat-extra-args]

This code (commit fe56188b07) is part of that used to check superblock block numbers specified on the command line, and the error reporting has become a little fuzzy relative to what we had previously.

Before:

root@debug1:~# debugfs -s 327b /dev/vdc
debugfs 1.42.8 (20-Jun-2013)
debugfs: Bad superblock number - 327b

After:

root@debug1:~# debugfs -s 327b /dev/vdc
debugfs 1.42.8 (20-Jun-2013)
debugfs: Bad block number - 327b
debugfs: Invalid block number: 327b
debugfs: Operation not permitted 

Both strtoblk() and parseulonglong() (which it calls) output error messages for bad/invalid block numbers, which is redundant in this case. The last (erroneous) error message is output by the call to com_err() which also causes the warning noted above.

Quoted 6 lines

It seems to me that the call to com_err() ought to be deleted, and maybe the immediately preceding call to strtoblk() ought to be converted to a call to parseulonglong() to restore the original messaging. I'd like to post a patch, but there are a number of other calls to strtoblk() in debugfs now that will produce two messages on errors, and the intent isn't clear to me. Is this just an area that needed a little more polish?

Back in October, I wrote a patch[1] that did a straight conversion to parse_ulonglong for 64bit support, but Lukas suggested using strtoblk instead. So I changed it, and that's what's in there now... uglified. Sorry about that. I should have argued more strongly against strtoblk. :(

I don't have a problem with changing it to parse_ulonglong.

[1] http://patchwork.ozlabs.org/patch/279295/

--D

#2 lore raw

* Darrick J. Wong <[email protected]>:

Quoted 48 lines · Eric Whitney
Quoted 40 lines

I've built the contents of the current e2fsprogs maint branch (4727c67dc2) and run make check on both a Pandaboard (ARM) and an x86-64 VM. In each case I used the following arguments to configure: --disable-uuidd --disable-libuuid --disable-libblkid.

Both the ARM and x86-64 runs produced warnings when compiling ea_refcount.c during make check. I posted a patch yesterday (e2fsck: fix printf conversion specs in ea_refcount.c) that clears those up for me on both platforms.

The ARM build and make check were otherwise clean.

The x86-64 build and make check contained one more problem - a warning while compiling debugfs.c:
../../debugfs/debugfs.c:2462:5: warning: too many arguments for format
[-Wformat-extra-args]

This code (commit fe56188b07) is part of that used to check superblock block numbers specified on the command line, and the error reporting has become a little fuzzy relative to what we had previously.

Before:

root@debug1:~# debugfs -s 327b /dev/vdc
debugfs 1.42.8 (20-Jun-2013)
debugfs: Bad superblock number - 327b

After:

root@debug1:~# debugfs -s 327b /dev/vdc
debugfs 1.42.8 (20-Jun-2013)
debugfs: Bad block number - 327b
debugfs: Invalid block number: 327b
debugfs: Operation not permitted 

Both strtoblk() and parseulonglong() (which it calls) output error messages for bad/invalid block numbers, which is redundant in this case. The last (erroneous) error message is output by the call to com_err() which also causes the warning noted above.

Quoted 6 lines

It seems to me that the call to com_err() ought to be deleted, and maybe the immediately preceding call to strtoblk() ought to be converted to a call to parseulonglong() to restore the original messaging. I'd like to post a patch, but there are a number of other calls to strtoblk() in debugfs now that will produce two messages on errors, and the intent isn't clear to me. Is this just an area that needed a little more polish?

Quoted 9 lines

Back in October, I wrote a patch[1] that did a straight conversion to parse_ulonglong for 64bit support, but Lukas suggested using strtoblk instead. So I changed it, and that's what's in there now... uglified. Sorry about that. I should have argued more strongly against strtoblk. :(

I don't have a problem with changing it to parse_ulonglong.

[1] http://patchwork.ozlabs.org/patch/279295/

Thanks for the background. Since Ted accepted the strtoblk() changes, maybe he's ok with the changed error reporting. For now, since I think he's interested in getting the next e2fsprogs maint release out the door, I posted a simple patch just to address the compile-time warning and that last broken error message. There are enough strtoblk() call sites that it's probably better to address them all and apply a common solution or not address any of them rather than to address just one.

Thanks,
Eric