1

Compiling an XS-module including libmba I cannot solve this warning with my beginners level experience in C:

helmut@Helmuts-MacBook-Air:~/github/LCS-XS$ make
"/Users/helmut/perl5/perlbrew/perls/perl-5.18.2/bin/perl"    "/Users/helmut/perl5/perlbrew/perls/perl-5.18.2/lib/5.18.2/ExtUtils/xsubpp"  -typemap "/Users/helmut/perl5/perlbrew/perls/perl-  5.18.2/lib/5.18.2/ExtUtils/typemap"  XS.xs > XS.xsc && mv XS.xsc XS.c
cc -c  -I. -fno-common -DPERL_DARWIN -fno-strict-aliasing -pipe - fstack-protector -I/usr/local/include -I/opt/local/include -O3   - DVERSION=\"0.01\" -DXS_VERSION=\"0.01\"  "- I/Users/helmut/perl5/perlbrew/perls/perl-5.18.2/lib/5.18.2/darwin- 2level/CORE"   XS.c
  XS.xs:55:26: warning: passing 'const void *' to parameter of type 'AV  *' (aka 'struct av *') discards qualifiers
    [-Wincompatible-pointer-types-discards-qualifiers]
       SV *line = *av_fetch(a, idx, 0);
                            ^
/Users/helmut/perl5/perlbrew/perls/perl-5.18.2/lib/5.18.2/darwin- 2level/CORE/embed.h:51:46: note: expanded from macro
'av_fetch'
#define av_fetch(a,b,c)         Perl_av_fetch(aTHX_ a,b,c)
                                                    ^
/Users/helmut/perl5/perlbrew/perls/perl-5.18.2/lib/5.18.2/darwin- 2level/CORE/proto.h:178:44: note: passing argument to
  parameter 'av' here
    PERL_CALLCONV SV**      Perl_av_fetch(pTHX_ AV *av, I32 key, I32 lval)
                                                   ^
1 warning generated.

The compiled module is working fine. But is there a way to code it without warning?

The relevant parts in LCS/XS.xs:

#include "EXTERN.h"
#include "perl.h"
#include "XSUB.h"

#include "ppport.h"

#include <string.h>

#include <mba/diff.h>
#include <mba/diff.c>

/* snipped */

inline
static const void * 
_av_idx(const void *a, int idx, void *context)
{
   //AV *av = a;
   SV *line = *av_fetch(a, idx, 0);
   STRLEN klen;
   char *key = SvPVbyte(line, klen);
   //printf("key: %s\n",key);

  return key;
}

/* snipped */

void lcs_LCS(obj, s1, s2)
  SV *obj
  AV * s1
  AV * s2

  PREINIT:
    struct CTX *ctx = (struct CTX *)SvIVX(SvRV(obj));

  PPCODE:
    int d, sn, i;
    struct varray *ses = varray_new(sizeof(struct diff_edit), NULL);

    IV n;
    IV m;
    n = av_top_index(s1);
    m = av_top_index(s2);

    // call libmba::diff()
    d = diff(s1, 0, n, s2, 0, m, &_av_idx, &_cmp_str,  NULL, 0, ses, &sn, NULL);

The part of mba/diff.h

typedef const void *(*idx_fn)(const void *s, int idx, void *context);

And in mba/diff.c:

int
diff(const void *a, int aoff, int n,
    const void *b, int boff, int m,
    idx_fn idx, cmp_fn cmp, void *context, int dmax,
    struct varray *ses, int *sn,
    struct varray *buf)
{

Is there a good practice to solve this warning without changing the source of libmba?

SOLVED:

inline
static const void * 
_av_idx(const void *a, int idx, void *context)
{
    SV *line = *av_fetch((AV *)a, idx, 0);
    //                   ^^^^^^
    STRLEN klen;
    char *key = SvPVbyte(line, klen);

    return key;
}
Helmut Wollmersdorfer
  • 441
  • 3
  • 12
  • `av_fetch((AV *)a, idx, 0);` should remove the warning. – wimh Jun 20 '15 at 10:14
  • @Wimmel: Thx, this works. I remember to have tried it before, but not in the final combination. – Helmut Wollmersdorfer Jun 20 '15 at 10:38
  • "Lie to the compiler and it will get its revenge." -- Henry Spencer ( http://www.lysator.liu.se/c/henry/ ) – pmg Jun 20 '15 at 10:47
  • true, see also [this example](http://stackoverflow.com/a/10906616/33499) – wimh Jun 20 '15 at 11:31
  • Don't know, what const exactly does, why the author of libmba defined it, and why the above solution (or workaround) should be a lie to the compiler. The whole module gets `a` as a reference to an array, and works on it read-only. Does this mean I should rewrite libmba? Yes, I will do it, but mostly for other reasons. – Helmut Wollmersdorfer Jun 20 '15 at 15:31
  • `const` is a promise by you, the programmer, to the compiler that the value will not change during evaluation of the function body. If you promise that but change the value anyway, that is the lie to the compiler. If `av_fetch()` does not change the value pointed to by the 1st parameter, then that function should have made it a `const` qualified parameter. I cannot guess why it hasn't. – pmg Jun 20 '15 at 15:49
  • I am the author of libmba. That const is supposed to emphasize that if your indexing routine were to modify the elements in any way, the diff routine could behave incorrectly. In this particular case, you are calling a function of a library that was not specifically crafted to be used as an indexing routine with diff and so it is no surprise that you have a minor incompatibility in the two APIs. If you know with certainty that av_fetch will not modify the elements, then you can assert that just by casting to `(AV *)`. So the correct solution in this case is to just cast to `(AV*)`. – squarewav Nov 05 '16 at 02:30

1 Answers1

0

Well ... in _av_idx you are promising you will not change the contents of the first parameter

inline static const void *_av_idx(const void *a, int idx, void *context)
//                                ^^^^^

But then you proceed to send that parameter to a function (av_fetch(a, idx, 0)) that does not promise to not change it. That makes your promise a little strange.

Just remove your promise ...

inline static const void *_av_idx(void *a, int idx, void *context)
//                       no const ^^^^^^^

Edit

Or you could copy the argument to a local variable and pass that

inline
static const void * 
_av_idx(const void *a, int idx, void *context)
{
   AV *a_copy;
   deep_copy(a_copy, a);
   if (a_copy != NULL) {
       SV *line = *av_fetch(a_copy, idx, 0);
       free(a_copy);
   } else {
       /* error */
   }
   STRLEN klen;
   char *key = SvPVbyte(line, klen);
   //printf("key: %s\n",key);

  return key;
}
pmg
  • 106,608
  • 13
  • 126
  • 198