floor of intval?

mikee

Hi developers.

Not that I want to start any flame nor pretend that I am a super-expert php coder but there is something that has caught my attention when reading over the new 2.4.4 version post.

As it is known one of the big problems that the developers have had to confront in the new version is the upgrade to php7 that is more strict thatn previous version. Regarding that point JWT wrote that "There have been a lot of PHP fixed going from PHP 5.6 to PHP 7.2. This change has exposed a lot of poorly-written PHP that was written in the past" and offer two examples of that. The second one (https://redmine.pfsense.org/projects/pfsense/repository/revisions/1ed21e0a2512554752de35d5758bcff503fcba9b/diff) replaces a floor(something-4) with a floor(intval(something)-4).

And this is my concern: does not an intval() casting convert anything inside to an int value already performing an implicit floor?

If the former is true then the new code is as good, or as bad, as the original. But the coders have EXPLICITELY (apart from parenthesis) added the intval so this is a DESIRED effect and now I am a bit puzzled with this construct.

Anyway thanks for your time and commitment.

Regards.

jimp

There may be a problem with that code (intval should probably be one paren over to the right from where it is...) but the floor is there because of the division. Even dividing an int by an int may not result in an int.

It should be, effectively doing: floor(int/int/int)-4, not a floor( (int/int/int) - 4) but the parenthesis are a bit messed up even in the first code.

mikee

Hi jimp.

I know and that is the reason I wrote the message first.

It is clear that dividing int by int may give a non-integer number and you can floor, ceil or truncate the result. And in doing that you still will have a float or double data type. Flooring, ceiling or truncating does not change the type of the data.

It is when you cast that you change the data type explicitely. But if you cast a non integer result then you are forcing an implicit truncation (or flooring depending on the compiler options). So casting is actually not needed inside any parenthesis. I think this is not a matter of parenthesis messing but of unuseful construct. Anyway there must be no collateral damage in doing what it is done as the final result will, most probably be, the same.

Perhaps it may be fine to cast to intval as an outer construct if you want to assess that the returned data is of type integer: intval(floor(int/int/int)-4) or even intval(floor(int/int/int))-4 as the 4 is already an integer by itself so there is no need to 'reconvert' it by casting.

jimp

The point of the intval() is not that, it is to ensure the contents of that variable are treated as an int. If it's unset/null PHP will barf on that. So it does an intval() on the variable to make sure it's dividing an integer by an integer (by an integer), and then subtracting four from that result and then taking the floor().

If you look at the follow-up commit from a few days later that fixed the syntax, you'll see what I mean: https://redmine.pfsense.org/projects/pfsense/repository/revisions/3e2e1b2c89f24bb201edc4d0c4b88e0644702b60/diff

mikee

Hi Jimp.

That reference clarify all. I did not know that the example code was only to make the point about the kind of existing coding and was not the final one.

Thanks for your time and explanations. Sorry for having bothered you all. It is clear that you know what you do :-).

Regards.