> On Feb. 15, 2017, 2:07 a.m., Joseph Wu wrote:
> > 3rdparty/stout/include/stout/duration.hpp, lines 99-100
> > <https://reviews.apache.org/r/56591/diff/2/?file=1633052#file1633052line99>
> >
> >     See: https://reviews.apache.org/r/53708/#comment229180

Note: There is a typo, I believe, in that suggested code. In this line:

```
t.tv_usec = (us() / MICROSECONDS) - (t.tv_sec * MILLISECONDS);
```

I think you want to suggest `ns() / MICROSECONDS` instead of `us() / 
MICROSECONDS`.

Let's also add a comment explaining why we're computing these quantities 
manually.


> On Feb. 15, 2017, 2:07 a.m., Joseph Wu wrote:
> > 3rdparty/stout/include/stout/protobuf.hpp, lines 178-180
> > <https://reviews.apache.org/r/56591/diff/2/?file=1633057#file1633057line178>
> >
> >     Hm...
> >     
> >     This is a `size_t` (unsigned int) to `int` implicit cast.  This means, 
> > if the string is of size 2^31 or greater, we will be passing in a negative 
> > size to the constructor.
> >     
> >     In proto2, the maximum size of a protobuf is 64 MB, so we're unlikely 
> > to hit the upper limit.  
> >     
> >     However, since it is possible to pass in an arbitrary string to this 
> > method, we should add:
> >     
> >     ```
> >     CHECK_LE(value.size(), std::numeric_limits<size_t>::max());
> >     ```
> >     ^ Possibly need a `static_cast<long>` in the first argument above.

Alright, good idea. We originally didn't do this because I figured it's safe 
for the reason you mentioned, but I agree that we should be extra cautious.


- Alex


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56591/#review165596
-----------------------------------------------------------


On Feb. 14, 2017, 9:05 a.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56591/
> -----------------------------------------------------------
> 
> (Updated Feb. 14, 2017, 9:05 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Stout: Removed MSVC compiler warnings.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/abort.hpp 
> b7dba032f579ead83f4e04c942239e721632eeb4 
>   3rdparty/stout/include/stout/duration.hpp 
> 0a30a09678c20937caa6f094c3c63a326e357932 
>   3rdparty/stout/include/stout/gzip.hpp 
> 7040a3275370e7447e843c2471f35e2ba26166e4 
>   3rdparty/stout/include/stout/os/windows/dup.hpp 
> 1fc4d55393aa617d1b67178c945ac0af91c30c99 
>   3rdparty/stout/include/stout/os/windows/environment.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/fd.hpp 
> fddaaa54deaea5e6ef3947142870c7fef76e76aa 
>   3rdparty/stout/include/stout/protobuf.hpp 
> c0f03bc547cddba29309c429b34a0c1e6015c8ea 
>   3rdparty/stout/include/stout/windows/os.hpp 
> b5172fca96c4151f4b1ebb6d343022558f45fc34 
> 
> Diff: https://reviews.apache.org/r/56591/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Reply via email to