On Sun, 20 Dec 2015 at 11:44 Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Vicentiu!

On Dec 19, Vicentiu Ciorbaru wrote:
> revision-id: 64149590c47d1cf6b1b227d8c90bdc23d20a8956 (mariadb-5.5.47-8-g6414959)
> parent(s): f89c9fc4b7b5d82c79775cb848225900b45a6b79
> author: Vicențiu Ciorbaru
> committer: Vicențiu Ciorbaru
> timestamp: 2015-12-19 14:14:10 +0200
> message:
>
> MDEV-7526: TokuDB doesn't build on OS X
>
> This patch fixes another compilation error caused by specifying
> attribute nonnull for all the parameters of the copyout function. This
> is incorrect as the function actually gets called with null parameters
> indirectly and thus only the output parameter should be nonnull.
>
> diff --git a/storage/tokudb/ft-index/util/dmt.h b/storage/tokudb/ft-index/util/dmt.h
> index d4b032f..43e44df 100644
> --- a/storage/tokudb/ft-index/util/dmt.h
> +++ b/storage/tokudb/ft-index/util/dmt.h
> @@ -679,16 +679,16 @@ class dmt {
>      __attribute__((nonnull))
>      void rebalance(subtree *const subtree);
>
> -    __attribute__((nonnull))
> +    __attribute__((nonnull(3)))
>      static void copyout(uint32_t *const outlen, dmtdata_t *const out, const dmt_node *const n);

Where is copyout called with the null parameter?
In 10.0 they've fixed it differently, it seems E.g.

The call stack that leads to null parameters for copyout:
in ft-index/util/dmt.cc

line 196:
int dmt<dmtdata_t, dmtdataout_t, dmtwriter_t>::insert(const dmtwriter_t &value, const dmtcmp_t &v, uint32_t *const idx) {
    r = this->find_zero<dmtcmp_t, h>(v, nullptr, nullptr, &insert_idx);

line 527:
int dmt<dmtdata_t, dmtdataout_t, dmtwriter_t>::find_zero(const dmtcmp_t &extra, uint32_t *const value_len, dmtdataout_t *const value, uint32_t *const idxp) const {
        r = this->find_internal_zero_array<dmtcmp_t, h>(extra, value_len, value, child_idxp);
 
line 966:
int dmt<dmtdata_t, dmtdataout_t, dmtwriter_t>::find_internal_zero_array(const dmtcmp_t &extra, uint32_t *const value_len, dmtdataout_t *const value, uint32_t *const idxp) const

This eventually calls copyout with nullptr arguments. 

        if (value != nullptr) {
            copyout(value, &this->d.a.values[best_zero]);
        }

May be we'd better use
the upstream fix?

I could not find the code snippet that you are referring to. I am looking at the file:


Here there seems to be no available fix. Did I look in the wrong place? The copyout function that I'm referring to has 3 or 4 parameters, not 2, as in your code snippet.

Regards,
Sergei
Chief Architect MariaDB
and security@mariadb.org
--
Vote for my Percona Live 2016 talks:
https://www.percona.com/live/data-performance-conference-2016/sessions/mariadb-connectors-fast-and-smart-new-protocol-optimizations#community-voting
https://www.percona.com/live/data-performance-conference-2016/sessions/mariadb-101-security-validation-authentication-encryption#community-voting