Build fail on Gentoo Linux

[PureTryOut]

Build fail on Gentoo Linux

[karlstav]

@relrod iniparser version 3.1 and 4.0 has the exact same arguments, so i don't know if a version detection is actually possible in a configure script. Would it be a sufficient workaround to use a compilation option?

So if you run something like:

./configure --legacy-iniparser

Would make it build with iniparser version 3.1

[relrod]

fwiw (@sedrubal you might be interested in this) for now I built this in a COPR:

https://copr.fedorainfracloud.org/coprs/codeblock/cava/

So: sudo dnf copr enable codeblock/cava && sudo dnf install cava should work on F22-24.

[karlstav]

@PureTryOut looks like you are using your system wide instalation of iniparser (not the one budled in cava). It fails with too many arguments in this function:

cava.c:193:3: error: too many arguments to function ‘iniparser_getseckeys’
   iniparser_getseckeys(ini, "eq", keys);
   ^
In file included from cava.c:52:0:
/usr/include/iniparser.h:142:9: note: declared here
 char ** iniparser_getseckeys(dictionary * d, char * s); 

In your version of iniparser the function iniparser_getseckeys takes two argements. However both the bundled version and the one on github takes three.

I suggest trying to uppgrade your version of iniparser.

[sedrubal]

I have the same issue on fedora:23 which ships version 3.1 (which is btw. the latest version on iniparser homepage ;) )

In version 4.0 they added the 3rd argument.

Can you somehow support version 3.1, too? I think this would be good, as fedora usually provides the latest version of packages in comparison to other distros and as even they provide only version 3.1, other distros won't be able to build cava with system's iniparser, too.

[karlstav]

Good idea @sedrubal, I think it can be done with the help of the config-script.

[PureTryOut]

Sorry I kinda forgot about this issue. Gentoo ships 3.1 as well, which explains the problem.

@sedrubal could you explain me how to edit ./configure so it'll use 3.1 properly?

[karlstav]

@PureTryOut as a workaround you can remove your installed version of iniparser. This will force cava to install the bundled version 4 of iniparser.

To create a permanent fix I will have to use the config script to detect the version. Then in the source I must create a separate code for version 3.1 that uses the old method of detecting number of keys. Just don't have the time right now.

[sedrubal]

Cool, thanks @karlstav

[PureTryOut]

Sadly that does not seem to work. It just starts complaining that it can't find iniparser.h...

[karlstav]

@PureTryOut did you run ./configure and make again after you uninstalled iniparser?

[PureTryOut]

Woops, I'm stupid. I forgot ./configure. It compiled and installed fine, but to run it I actually had to install iniparser again. It now runs fine though.

[relrod]

@karlstav any update here? Wanting to possibly packaging this for Fedora (I saw the specfile in the readme, I was thinking of submitting it as an actual package though) and it would have to use the system iniparser, but I'm hitting this same error.

[karlstav]

@relrod i started looking for a way to detect (in the config script) iniparser version or whether the function iniparser_getseckeys takes two or three arguments. But I'm stuck... Maybe i should post an issue at the iniparser github page?

[sedrubal]

@relrod the copr build works on fedora 24 with fedoras iniparser. Thanks so far ;)

[relrod]

@karlstav Yep, that would work!

[relrod]

@sedrubal Yeah, I patched it in the specfile, not ideal but it works for now :)

[karlstav]

created a separate branch for this iss83, you can test it already, not sure if it works.

[karlstav]

also test if the equalizer actually works with this. (that is the only thing this problematic function (iniparser_getseckeys) is actually used for)

[relrod]

@karlstav

cava.c:216:8: error: assignment to expression with array type
   keys = iniparser_getseckeys(ini, "eq");
        ^

In the Fedora COPR package, I did this:

--- cava.c.orig 2016-06-15 17:05:34.548834878 -0400
+++ cava.c      2016-06-15 17:05:09.976739403 -0400
@@ -208,8 +208,8 @@
        smcount = iniparser_getsecnkeys(ini, "eq");
        if (smcount > 0) {
                smooth = malloc(smcount*sizeof(*smooth));
-               const char *keys[smcount];
-               iniparser_getseckeys(ini, "eq", keys);
+               char *keys[smcount];
+               memcpy(keys, iniparser_getseckeys(ini, "eq"), smcount*sizeof(*smooth));
                for (int sk = 0; sk < smcount; sk++) {
                        smooth[sk] = iniparser_getdouble(ini, keys[sk], 1);
                }

...although I'm not 100% sure the last argument to memcpy is correct. I'm not a C coder by trade ;)

[karlstav]

Try it now, I changed it to match the Fedora COPR package.

It looks strange to use memcpy, but it might be because there was a problem with memory leak in the iniparser_getseckeys function. Which was why they added the third argument.

[relrod]

@karlstav well in C, functions can't return arrays, so I think memcpy is the only way without passing the pointer directly to the function wanting to return an array (like they do in the new iniparser). But again I'm not a C coder by trade, so I could be off here.

That change did work and builds cleanly for me. :) But before you just match the COPR package -- you should probably double check the smcount*sizeof(*smooth) parameter to memcpy - That was a guess on my part, I'm not sure if it's right. If it is, then +1 on this patch, it works well for me.

[karlstav]

I'm not 100% on memory allocation myself, but I think this might be sufficient:

char **keys = iniparser_getseckeys(ini, "eq");

the function returns a pointer to statically allocated character string. So I don't think any memory allocation is necessary. I made the change so you can test it.

[relrod]

Works fine for me! @karlstav thanks for this fix and for cava, it's awesome!