Hi Jc,

Calculation of the required buffer size ("old fashion way") looks error-prone. I don't think we need to care about ancient SUSv2, I suppose C99 is supported by all (or almost all) compilers.

But if you want to keep the code SUSv2-compatible, you can do something like
  char dummy = 0;
  int requiredSize = snprintf(&dummy, sizeof(dummy), formatString,...);

Other note:
"if (obtained_size == len) " is useless
obtained_size is a result of snprintf(result, len, ...)
and snprintf returns number of chars _excluding_ terminating null, so obtained_size is always less than len.

--alex

On 08/10/2018 07:55, JC Beyler wrote:
Hi all,

Anybody motivated to look at this change? :)

Webrev: http://cr.openjdk.java.net/~jcbeyler/8201224/webrev.01/ <http://cr.openjdk.java.net/%7Ejcbeyler/8201224/webrev.01/>
Bug: https://bugs.openjdk.java.net/browse/JDK-8201224

Let me know what you think!
Jc


On Wed, Aug 8, 2018 at 11:34 AM JC Beyler <jcbey...@google.com <mailto:jcbey...@google.com>> wrote:

    Hi all,

    Here is a revised webrev that only does the stated fix in the bug. I
    filed https://bugs.openjdk.java.net/browse/JDK-8209153 for fixing
    the style.

    Webrev: http://cr.openjdk.java.net/~jcbeyler/8201224/webrev.01/
    <http://cr.openjdk.java.net/%7Ejcbeyler/8201224/webrev.01/>
    Bug: https://bugs.openjdk.java.net/browse/JDK-8201224

    Let me know what you think!

    Thanks!
    Jc

    On Fri, Aug 3, 2018 at 9:12 AM JC Beyler <jcbey...@google.com
    <mailto:jcbey...@google.com>> wrote:

        Yes that was my train of thought as well (do it in various
        webrevs and via various bugs for tracking). Is there a will or
        interest to doing a few passes to just put these tests into the
        same format once and for all? If so, I'll create a few bugs for
        a few cleanup webrevs.

        On the other hand, often there is a worry that we lose history
        due to the cleanup passes modifying each line of each file.

        Hence me asking before doing something like that :-)
        Jc

        On Fri, Aug 3, 2018 at 7:45 AM Daniel D. Daugherty
        <daniel.daughe...@oracle.com
        <mailto:daniel.daughe...@oracle.com>> wrote:

             > Ps: A lot of these tests seem to be in GCC style (where
            spaces are all
             > over the place, 4 space indent, etc.), should we do a
            one-pass per
             > folder to make it into more standard hotspot style?

            If you do whitespace/style cleanups, please do them with a
            separate bug.

            Dan



            On 8/3/18 12:11 AM, JC Beyler wrote:
            Hi all,

            Small change to the locationToString method to make the
            string dynamically generated.

            I added a bit of paranoia to handle the case where things
            might be changed so that an error would propagate to the test.

            Igor asked me to also at least fix the NSK_VERIFY tests in
            the file.

            Let me know what you think:

            Webrev:
            http://cr.openjdk.java.net/~jcbeyler/8201224/webrev.00/
            <http://cr.openjdk.java.net/%7Ejcbeyler/8201224/webrev.00/>
            Bug: https://bugs.openjdk.java.net/browse/JDK-8201224

            Thanks,
            Jc

            Ps: A lot of these tests seem to be in GCC style (where
            spaces are all over the place, 4 space indent, etc.),
            should we do a one-pass per folder to make it into more
            standard hotspot style?



--
        Thanks,
        Jc



--
    Thanks,
    Jc



--

Thanks,
Jc

Reply via email to