extfs: hp48: fix float truncation

[mooffie]

[This is forked from #3729.]

The hp48 helper uses the following code to convert floats (of which filesizes are) to integers:

hp48_retsize()
{
    printf "%d" "$2" 2>/dev/null
}
# Note: The number is in $2, not in $1.

(This results in truncation, not rounding.)

While this trick is acceptable according to the a ​standard ("If an argument operand cannot be completely converted [...] the utility shall [...] write the value accumulated at the time the error was detected to standard output"), it seems that some shells don't support it. @Andrew gives ​as example a certain version of Bash.

So we'd better change that code. One possibility:

hp48_retsize()
{
    echo ${2%.*}
}

(We shouldn't do "printf %.0f NUMBER" as this would round the number, not truncate it. Not that rounding is a sin, but we should better stick to the current behavior.)

[mooffie]

(We shouldn't do "printf %.0f NUMBER" as this would round the number, not truncate it. Not that rounding is a sin, but we should better stick to the current behavior.)

(While I haven't seen anything other than ".5" used as the fraction in the two samples I found on the net, these samples were very short and I don't know the meaning of this fraction.)

I'm attaching a patch.

[zaytsev]

To be honest, I feel bad about the replacement, because it will obviously not handle all kinds of badness like .2, 1.2e3 and such. Of course, this doesn't make too much sense, but you said it yourself that we don't have a lot of valid inputs to test against.

How about using awk instead? It is used in the script anyways, and has a handy int() function. I'm not an awk expert, but I would imagine that something like awk '{ print int($1) }' would do the trick.

[mooffie]

Replying to zaytsev:

To be honest, I feel bad about the replacement, because it will obviously not handle all kinds of badness like .2, 1.2e3 [...] something like awk '{ print int($0) }' would do the trick.

I tried your suggestion, but I couldn't bring myself to upload the patch (I had two versions of it). Here's why:

The current code is ridiculous as it is. Making hp48_retsize() use awk will make it worthy of appearing in the "Software" section of Wikipedia's "Farce" article. I'm not trying to be "clever" or "witty": I've been struggling with uploading that patch for the last 24 hours.

So I see two possibilities (beside someone else uploading that patch):

• Use my previous patch (which matches the original behavior, btw).

• Use the following patch, which makes the inside of the loop sane. I intentionally left the silly $INPUT and "EOF" things in (instead of simply doing "while read obj_name obj_size obj_type") because I felt changing this wouldn't be appreciated considering the circumstances (lack of exhaustive tests). I know it's not what you were expecting. To me this looks like the obvious right choice, to get rid of silly code as we go along, but it's fine with me if you reject it.

[mooffie]

And next is a follow up patch to make the code more readable (only if you choose the "--ALTERNATIVE" patch.)

(BTW, I tested the helper with a "Directory" line to make sure the corresponding printf still works.)

[zaytsev]

On an unrelated note: could you please enable automatic sign-off for your patches?

git config [--global] format.signoff true

I've just realized that I've recently committed some of them without amending with my sign-off (so, in the end, they haven't got any), and now I feel bad about it. It's not the end of the world, but still, if we are doing it at all, we should be doing this consistently. Thanks!

[zaytsev]

The current code is ridiculous as it is.

I agree that this old code is a joke and yes, I'd still be wary of making changes to it due to the level of test coverage, but whatever, first, there is a basic test now, so outrageous brokenness will hopefully fail it, and then it didn't even work before you fixed it, so...

Branch: 3747_hp48_extfs
Initial changeset: 6ecb6d12e120b692c93d39ff610cb26728da8867

[zaytsev]

Merged to master: [94787407f14e4d4bcf54ffbe811da64ba5803266].