On 01/14/2017 05:51 PM, J William Piggott wrote: > > > On 01/14/2017 04:34 AM, Sami Kerola wrote: >> On Thu, 12 Jan 2017, J William Piggott wrote: >>> On 01/11/2017 04:44 PM, Sami Kerola wrote: >>>> On Sun, 8 Jan 2017, J William Piggott wrote: >>>>>> @@ -1031,9 +1032,7 @@ calculate_adjustment(const struct hwclock_control *ctl, >>>>>> exact_adjustment = >>>>>> ((double)(systime - last_time)) * factor / (24 * 60 * 60) >>>>>> + not_adjusted; >>>>>> - tdrift_p->tv_sec = (int) exact_adjustment; >>>>>> - if (exact_adjustment < 0) >>>>>> - tdrift_p->tv_sec--; >>>>>> + tdrift_p->tv_sec = (int) floor(exact_adjustment); >>>>> >>>>> Is the cast required now? >>>> >>>> floor(3) will return double, tv_sec is time_t. Yes I should and have >>>> fixed the cast from int to time_t. >>> >>> Isn't the cast implied by assignment and done automatically? Isn't that >>> why the previous mismatch with the (int) cast worked? In the next line >>> of code we assign a double to tv_usec which is a mismatch also. >>> >>> I built your latest branch with and without the cast and the results >>> were binary identical. >> >> Preferably there should not be casts at all. But sometimes they are hard >> to avoid, and then one should do as few casts as possible to avoid cutting >> bits out. >> >> Think casting in terms of shape sorting toy. The more data is casted >> through none-matching types the more the original shape is modified to fit >> to storage where it is pushed. >> >> That is also the reason why casting to time_t in this case makes sense. >> While it might be that right now, on systems we happen to use, int and >> time_t are the same type there are no guarantees about other systems and >> future. In practical terms; casting exactly to matching storage type is >> the right way to avoid damaging data. >> > > What I'm trying to say is, assignment operators automatically convert > the right operand to the type of the left operand; making your cast > unnecessary. Try it, build hwclock with the cast. Now, remove the cast > and build it. The resulting binaries are bit for bit identical. > > ISO C11: > The type of an assignment expression is the type the left operand would have > after lvalue conversion. > > In simple assignment (=), the value of the right operand is converted to > the type of the assignment expression and replaces the value stored in > the object designated by the left operand. > > It wasn't the intent of my previous message to be offensive in any way, Sami. I was only trying to state my position using different language, so as not to simply repeat myself. Also, I'm not continuing this topic to be argumentative. The reason is for my own education. If my opinion is wrong, that is fine; I would like to understand why, so as to improve my own coding. As I further read the ISO C standard, I'm more convinced of my belief. It seems to me the compiler is required to use the type conversion routine associated with assignment operators, because it must yield two different results. A /qualified/ left operand type is stored, but the evaluation of the assignment expression returns an /unqualified/ left operand type. Since using the assignment operator's type conversion routine is required (if I'm correct), the compiler would ignore any cast on the results of the right hand operand for code optimization, yes? That seems to be true for my compiler, as the cast makes no difference in the resulting binary. If anyone knows the correct answer to this, I am very interested in learning it. Thank you in advance. >>>> ---------------------------------------------------------------- >>>> The following changes since commit 89958178f6d6ebe0944d423feaea66be521fff43: >>>> If mtab support is disabled, disable ro/rw mtab checks (2017-01-10 17:32:32 +0100) >>>> are available in the git repository at: >>>> git://github.com/kerolasa/lelux-utiliteetit.git hwclock-jwp-reviewed >>>> for you to fetch changes up to d3e33557e6794446c712ca264e54fe4b186a17ca: >>>> hwclock: remove --compare option (2017-01-11 21:04:36 +0000) >>>> ---------------------------------------------------------------- >>>> >>>> Thank you JWP for reviews, I hope I did not miss anything but if I did let >>>> me know and I will follow up. Even better if everyone agrees that this is >>>> now good enough to be merged, and we can move forward with other updates >>>> such as adjtimex(8) work. >>> >>> Thanks for all the work, Sami. I've been testing your latest branch and >>> haven't found any problems. I cannot test all of your changes though, for >>> example I don't have libaudit installed. For what it's worth, I think it >>> is ready. >> >> Good point. I do not have libaudit either. Hearing that setup does not >> have functional problems would really nice. >> > -- > To unsubscribe from this list: send the line "unsubscribe util-linux" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe util-linux" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
