maria-developers team mailing list archive

[Vicențiu Ciorbaru <vicentiu@xxxxxxxxxxx>]

On Sun, 20 Dec 2015 at 11:44 Sergei Golubchik <serg@xxxxxxxxxxx> 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:

https://github.com/Tokutek/ft-index/blob/master/util/dmt.cc

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@xxxxxxxxxxx
> --
> 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
>