RFR(xxs): 8167650: NMT should check for invalid MEMFLAGS.

Thomas Stüfe thomas.stuefe at gmail.com
Wed Nov 23 14:15:45 UTC 2016


Hi Zhengyu,

thanks for looking at this. See my answer to David, I did the last rev as
you guys requested. See: http://cr.openjdk.java.net/~stuefe/webrevs/
8167650-NMT-should-check_MEMFLAGS/webrev.03/webrev/

Kind Regards, Thomas

On Wed, Nov 23, 2016 at 2:58 PM, Zhengyu Gu <zgu at redhat.com> wrote:

> Sorry for participating late, I did not expect to be a lengthy discussion.
>
>
>
> On 11/23/2016 06:20 AM, David Holmes wrote:
>
>> Hi Thomas,
>>
>> On 23/11/2016 5:29 PM, Thomas Stüfe wrote:
>>
>>> Hi David,
>>>
>>> On Wed, Nov 23, 2016 at 3:08 AM, David Holmes <david.holmes at oracle.com
>>> <mailto:david.holmes at oracle.com>> wrote:
>>>
>>>     Hi Thomas,
>>>
>>>         David did not like that, he thought it strange to check an enum
>>> for
>>>         correct enum values. I can see his point. He would have preferred
>>>         instead an assert inside NMTUtil::flag_to_index() which before
>>>         returning checks the to-be-returned index for
>>>         0>=i>=mt_number_of_types. I did not like that because strictly
>>>         speaking inside of NMTUtil::flag_to_index() it is not known that
>>> the
>>>         returned integer will be used by the caller as an index into an
>>>         array
>>>         of mt_number_of_types length.
>>>
>>>
>>>     Sorry but how is NMTUtil::flag_to_index anything but a mapping from
>>>     0 to mt_number_of_types ??? MEMFLAGS aka MemoryType is an enum that
>>>     spans zero to mt_number_of_types! We're validating the post
>>>     condition for the method.
>>>
>>>
>>> Sorry, this may be clear for you but was not clear for me. There is no
>>> comment at NMTUtil::flag_to_index explaining the range of the output
>>> value, and the callers are spread over multiple files. The fact that the
>>> original author found it necessary to add this conversion function and
>>> to mask the upper bits out suggested to me that the returned index is
>>> something different than MEMFLAGS, or may be different in the future.
>>> Otherwise he could have just used the MEMFLAGS enum itself as index.
>>>
>>
>> Can't answer that - perhaps just for cleanliness of the type system?
>> Perhaps he thought the enum was strongly-typed? Or perhaps it was to hide
>> the actual mapping function to allow for a different scheme.
>>
>
> Yes, I was assuming the type system should ensure correct MEMFLAGS value
> here.
>
>
>
>> If anything the & 0xff suggests that the input might be something other
>> than a MEMFLAGS value - despite it being typed as MEMFLAGS - that needs to
>> be brought back into range.
>>
>> Note that I still do not understand the & 0xff part. Once MEMFLAG values
>>> surpass 0xff, what is supposed to happen?
>>>
>>
>> I don't think there is any expectation that we will ever go that high:
>>
>> enum MemoryType {
>>   // Memory type by sub systems. It occupies lower byte.
>>
>> though why this was restricted to be a byte-size value I can't say. I
>> also don't know why we then typedef MemoryType to MEMFLAGS.
>>
>
> NMTUtil::flag_to_index() converts flag to byte size for encoding the value
> into malloc header (services/mallocTracker.hpp).
>
> At the time of implementation, we don't have some types, such as u1, etc.
> maybe change the return type to u1 (maybe refactor MallocHeader)
> to make the code more explicit.
>
>
>>     As I said on Oct 19, and which Chris indicated he agreed with, it
>>>     makes no sense to me to put the same assert in all the callers of
>>>     flag_to_index instead of doing the validation inside flag_to_index.
>>>
>>>
>>> I understand you. I understood Max' comment otherwise. Faced with two
>>> opinions, and as a outsider having no way to weight them against each
>>> other, I have to choose one.
>>>
>>>
>> Fair enough, but Chris made it 2-1. :)
>>
>
> I am also with David and Chris. mt_number_of_types just for working around
> imperfect of enum type system,
> I would rather it not to leak everywhere.
>
> Thanks,
>
> -Zhengyu
>
>
>
>>     Whether you choose to validate the flag or the index I don't care.
>>>
>>>
>>> Then we could have just gone with my very first webrev and I could have
>>> saved work. In your very first answer to my original webrev you wrote:
>>> "I'm a little perplexed with asserting that something of MEMFLAGS type
>>> must be an actual MEMFLAGS value". I understood this as objection.
>>>
>>
>> Yes but I've now reached a point where I'd accept that if it is the only
>> way to have the assertion inside the function. When I made that objection I
>> did not expect this to take the path it did. Sorry.
>>
>> I am fine with either solution. I am also fine with someone else fixing
>>> this issue. My only preference would be to some form of assert in this
>>> code, because I spent quite some time to find the memory overwriter this
>>> caused.
>>>
>>
>> My preference, obviously, is an assert inside flag_to_index, on the value
>> of index to be returned.
>>
>> I did not expect this to become so prolonged, and I'm sorry about that,
>> but I really object to the callers of the function validating its output.
>>
>> Thanks,
>> David
>>
>>     If we were in a decent language the bug would have been impossible
>>>     to begin with.
>>>
>>>     David
>>>     -----
>>>
>>>
>>> Thanks, Thomas
>>>
>>>
>>>
>>>     On 22/11/2016 1:21 AM, Thomas Stüfe wrote:
>>>
>>>         Hi all,
>>>
>>>         this small issue got fc extension, so lets wrap this up. Thanks
>>> to
>>>         all for reviewing!
>>>
>>>         New webrev:
>>>
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-shoul
>>> d-check_MEMFLAGS/webrev.02/webrev/
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-shou
>>> ld-check_MEMFLAGS/webrev.02/webrev/>
>>>
>>>          No changes to the last one but the updated copyrights Chris did
>>> ask
>>>         for. Solution still follows Max' suggestion of checking the index
>>>         right before consumption.
>>>
>>>         Thanks, and Kind Regards, Thomas
>>>
>>>
>>>         On Mon, Nov 21, 2016 at 4:17 PM, Thomas Stüfe
>>>         <thomas.stuefe at gmail.com <mailto:thomas.stuefe at gmail.com>
>>>         <mailto:thomas.stuefe at gmail.com
>>>         <mailto:thomas.stuefe at gmail.com>>> wrote:
>>>
>>>         Hi Chris,
>>>
>>>         On Thu, Nov 17, 2016 at 8:54 PM, Chris Plummer
>>>         <chris.plummer at oracle.com <mailto:chris.plummer at oracle.com>
>>>         <mailto:chris.plummer at oracle.com
>>>         <mailto:chris.plummer at oracle.com>>> wrote:
>>>
>>>         On 10/18/16 11:49 PM, David Holmes wrote:
>>>
>>>
>>>
>>>         On 19/10/2016 3:17 PM, Thomas Stüfe wrote:
>>>
>>>
>>>
>>>         On Wed, Oct 19, 2016 at 3:21 AM, David Holmes
>>>         <david.holmes at oracle.com <mailto:david.holmes at oracle.com>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com
>>> >>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com
>>> >>>>
>>>         wrote:
>>>
>>>         On 18/10/2016 3:39 PM, Thomas Stüfe wrote:
>>>
>>>         Hi David, Max,
>>>
>>>         I changed the asserts according to Max' suggestion. Instead of
>>>         checking inside flag_to_index, now I check before callers of this
>>>         function use this value to access memory.
>>>
>>>
>>>         I don't see where Max suggested that??
>>>
>>>
>>>         Max wrote: " I think the decision on whether to access a slot
>>> should
>>>         occur as close to memory accessing code as possible." and
>>> proceeded
>>>         to suggest fixing VirtualMemorySnapshot::by_type() as well.
>>>
>>>
>>>         I did not interpret that comment that way, and was puzzled by the
>>>         reference to by_type.
>>>
>>>
>>>         It doesn't make sense to me to have all the callers of
>>> flag_to_index
>>>         check what it returned instead of doing it inside flag_to_index.
>>>
>>>
>>>         I disagree. Imho it makes sense to either check the Memflags
>>>         enumeration input argument in flag_to_index() or the returned
>>> index
>>>         before consumption. In both cases one knows the valid value
>>> range.
>>>         Strictly speaking checking the index in flag_to_index() cannot be
>>>         done because it is a faceless int type whose valid values are
>>>         not yet
>>>         known.
>>>
>>>
>>>         The index has to fall in the range 0 <= index <=
>>> mt_number_of_types,
>>>         and I was suggesting that it makes more sense to verify this
>>> once in
>>>         flag_to_index() than in all the callers of flag_to_index.
>>>
>>>         Hi Thomas,
>>>
>>>         Just catching up on this thread. This is the same conclusion I
>>> came
>>>         to. I don't understand what you mean by "it is a faceless int
>>> type
>>>         whose valid values are not yet known".
>>>
>>>
>>>         Thank you for looking into this!
>>>
>>>         "it is a faceless int type whose valid values are not yet known"
>>> was
>>>         maybe expressed sloppily:
>>>
>>>         My first patch checked the input enum "flag" argument inside
>>>         NMTUtil::flag_to_index() for correct enum values :
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-shoul
>>> d-check_MEMFLAGS/webrev.00/webrev/
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-shou
>>> ld-check_MEMFLAGS/webrev.00/webrev/>
>>>
>>>
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-shou
>>> ld-check_MEMFLAGS/webrev.00/webrev/
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-shou
>>> ld-check_MEMFLAGS/webrev.00/webrev/>>
>>>
>>>
>>>         David did not like that, he thought it strange to check an enum
>>> for
>>>         correct enum values. I can see his point. He would have preferred
>>>         instead an assert inside NMTUtil::flag_to_index() which before
>>>         returning checks the to-be-returned index for
>>>         0>=i>=mt_number_of_types. I did not like that because strictly
>>>         speaking inside of NMTUtil::flag_to_index() it is not known that
>>> the
>>>         returned integer will be used by the caller as an index into an
>>>         array
>>>         of mt_number_of_types length.
>>>
>>>         It is all extreme nitpicking and in the end amounts to the same
>>> :)
>>>
>>>         The current patch does neither, but follows Max' suggestion of
>>>         checking the index right before it is consumed to access an
>>> array. I
>>>         think this is a good solution and very clear.
>>>
>>>
>>>
>>>         BTW, I'll sponsor this fix for you once it is finalized. Please
>>>         update the copyright dates first.
>>>
>>>
>>>         Thank you. I'll update the webrev and repost.
>>>
>>>
>>>
>>>         thanks,
>>>
>>>         Chris
>>>
>>>
>>>         Kind Regards, Thomas
>>>
>>>
>>>
>>>         David
>>>
>>>         It is all academical and mostly a matter of taste.
>>>
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-shoul
>>> d-check_MEMFLAGS/webrev.01/webrev/index.html
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-shou
>>> ld-check_MEMFLAGS/webrev.01/webrev/index.html>
>>>
>>>
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-shou
>>> ld-check_MEMFLAGS/webrev.01/webrev/index.html
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-shou
>>> ld-check_MEMFLAGS/webrev.01/webrev/index.html>>
>>>
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-shou
>>> ld-check_MEMFLAGS/webrev.01/webrev/index.html
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-shou
>>> ld-check_MEMFLAGS/webrev.01/webrev/index.html>
>>>
>>>
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-shou
>>> ld-check_MEMFLAGS/webrev.01/webrev/index.html
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-shou
>>> ld-check_MEMFLAGS/webrev.01/webrev/index.html>>>
>>>
>>>
>>>         As David correctly writes, this is technically not a bug, so I
>>> guess
>>>         this will have to wait until java 10.
>>>
>>>
>>>         Yes, afraid so.
>>>
>>>
>>>         The fix is trivial and I will try to get fc extension for this
>>> (now
>>>         that Goetz explained to me how to do this :). It seems this is
>>> done
>>>         for many other non-bug issues as well.
>>>
>>>         ..Thomas
>>>
>>>
>>>         Thanks, David
>>>
>>>         Kind Regards, Thomas
>>>
>>>
>>>
>>>         On Fri, Oct 14, 2016 at 12:57 AM, David Holmes
>>>         <david.holmes at oracle.com <mailto:david.holmes at oracle.com>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com
>>> >>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com
>>> >>>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com
>>> >>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com
>>> >>>>>
>>>         wrote:
>>>
>>>         On 13/10/2016 10:53 PM, Thomas Stüfe wrote:
>>>
>>>         Hi David,
>>>
>>>         On Thu, Oct 13, 2016 at 12:08 PM, David Holmes
>>>         <david.holmes at oracle.com <mailto:david.holmes at oracle.com>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com
>>> >>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com
>>> >>>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com
>>> >>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com
>>> >>>>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com
>>> >>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com
>>> >>>
>>>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com
>>> >>
>>>         <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com>
>>>         <mailto:david.holmes at oracle.com
>>> <mailto:david.holmes at oracle.com>>>>>>
>>>
>>>         wrote:
>>>
>>>         Hi Thomas,
>>>
>>>         On 13/10/2016 3:49 PM, Thomas Stüfe wrote:
>>>
>>>         Hi all,
>>>
>>>         may I have plase a review for this tiny change? It just adds some
>>>         assert to NMT.
>>>
>>>         Bug: https://bugs.openjdk.java.net/browse/JDK-8167650
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650>
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650
>>> <https://bugs.openjdk.java.net/browse/JDK-8167650>>
>>>
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650>
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650
>>> <https://bugs.openjdk.java.net/browse/JDK-8167650>>>
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650>
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650
>>> <https://bugs.openjdk.java.net/browse/JDK-8167650>>
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650>
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650
>>> <https://bugs.openjdk.java.net/browse/JDK-8167650>>>>
>>>
>>>
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650>
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650
>>> <https://bugs.openjdk.java.net/browse/JDK-8167650>>
>>>
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650>
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650
>>> <https://bugs.openjdk.java.net/browse/JDK-8167650>>>
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650>
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650
>>> <https://bugs.openjdk.java.net/browse/JDK-8167650>>
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650>
>>>         <https://bugs.openjdk.java.net/browse/JDK-8167650
>>> <https://bugs.openjdk.java.net/browse/JDK-8167650>>>>> webrev:
>>>
>>>
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_>
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_>>
>>>
>>>
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_ <
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_>
>>>
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_>>>
>>>
>>>
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_>
>>>
>>>
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_ <
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_>>
>>>
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_>
>>>
>>>
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_ <
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_>>>>
>>>
>>>
>>>
>>>
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_>
>>>
>>>
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_ <
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_>>
>>>
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_>
>>>
>>>
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_ <
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_>>>
>>>
>>>
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_>
>>>
>>>
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_ <
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_>>
>>>
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_>
>>>
>>>
>>> <http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_ <
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8167650-NMT-should-check_>>>>>
>>>
>>>
>>>         MEMFLAGS/webrev.00/webrev/
>>>
>>>         We had an ugly memory overwrite caused by this - ultimately our
>>>         fault, because we fed an invalid memory flag to NMT - but it was
>>>         difficult to find. An assert would have saved some time.
>>>
>>>
>>>         I'm a little perplexed with asserting that something of MEMFLAGS
>>>         type
>>>         must be an actual MEMFLAGS value - it implies the caller is
>>> coercing
>>>         plain int to MEMFLAGS, and I don't have much sympathy if they
>>> mess
>>>         that up. Can't help wondering if there is some clever C++ trick
>>> to
>>>         flag bad conversions at compile-time?
>>>
>>>
>>>         The error was caused by an uninitialized variable of type
>>> MEMFLAGS.
>>>         This was our fault, we have heavily modified allocation.hpp and
>>>         introduced an error then merging changes from upstream. Due to a
>>>         merging error this lead to a case where Arena::_flags was not
>>>         initialized and contained a very large value.
>>>
>>>
>>>         Ah I see. Lack of default initialization can be annoying :)
>>>
>>>         I admit it looks funny. If it bothers you, I could instead check
>>> the
>>>         returned index to be in the range for the size of the _malloc
>>> array
>>>         in MallocMemorySnapshot::by_type(). Technically, it would mean
>>> the
>>>         same.
>>>
>>>
>>>         So I just realized that here:
>>>
>>>         62   // Map memory type to human readable name 63 static const
>>>         char* flag_to_name(MEMFLAGS flag) { 64     assert(flag >= 0 &&
>>> flag
>>>         < mt_number_of_types, "Invalid flag value %d.", (int)flag); 65
>>>         return _memory_type_names[flag_to_index(flag)]; 66   }
>>>
>>>         we call flag_to_index, so the assert is redundant as it is
>>>         already in
>>>         flag_to_index. Then presumably we change flag_to_index to
>>> something
>>>         like this:
>>>
>>>         static inline int flag_to_index(MEMFLAGS flag) { int index =
>>> (flag &
>>>         0xff); assert(index >= 0 && index < mt_number_of_types, "Invalid
>>>         flag
>>>         value %d.", (int)flag); return index; }
>>>
>>>         so we're validating the index rather than the flag.
>>>
>>>         Cheers, David
>>>
>>>
>>>
>>>
>>>         The function that takes the index should validate the index, so
>>> that
>>>         is fine.
>>>
>>>         Which one were you actually passing the bad value to? :)
>>>
>>>         This isn't a strong objection just musing if we can do better.
>>>         And as
>>>         the hs repos are still closed, and likely to remain so till early
>>>         next week, we have some slack time :)
>>>
>>>
>>>         :) Sure.
>>>
>>>         Kind Regards, Thomas
>>>
>>>
>>>         Cheers, David
>>>
>>>         Thank you!
>>>
>>>         Thomas
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>


More information about the hotspot-runtime-dev mailing list