libpar2 and par2 fail 'make check' with clang++

[dinge345@…]

libpar2 does not compile:

:info:build ./par2fileformat.h:87:25: error: flexible array member 'entries' of non-POD element type 'FILEVERIFICATIONENTRY []'

but forcing compiler to g++ works:

port install libpar2 configure.compiler=gcc

Using OS X 10.7.2 with XCode 4.2, no older XCode installed. MacPorts defaults to clang in this configuration.

[dinge345@…]

/opt/local/var/macports/logs/_opt_local_var_macports_sources_rsync.macports.org_release_tarballs_ports_archivers_libpar2/libpar2/main.log

[ryandesign (Ryan Carsten Schmidt)]

I don't know what "configure.compiler=gcc" means, since Xcode 4.2 does not include any version of gcc; please try with "configure.compiler=llvm-gcc-4.2" instead; if that works, we want to use that.

[dinge345@…]

Yes, it works with "configure.compiler=llvm-gcc-4.2".

[dinge345@…]

Unsurprisingly I get the same error with the par2 port, it also works with llvm-gcc-4.2

[ryandesign (Ryan Carsten Schmidt)]

Replying to dinge345@…:

Yes, it works with "configure.compiler=llvm-gcc-4.2".

Thanks; fixed in r86473.

Replying to dinge345@…:

Unsurprisingly I get the same error with the par2 port

#31789

[jeremyhu (Jeremy Huddleston Sequoia)]

Reopening to track a real fix rather than a workaround

[jeremyhu (Jeremy Huddleston Sequoia)]

This is essentially the same as #33152

[jeremyhu (Jeremy Huddleston Sequoia)]

Andrea,

You found the solution for the similar issue in ppl, so would you mind fixing par2 and libpar2 as well? I'm fluent in C++, and it doesn't look like this is the same root cause (issues in configure).

If this is a deficiency in clang++, we should file a report against the compiler rather than working around it.

[jeremyhu (Jeremy Huddleston Sequoia)]

Cc Me!

[anddam (Andrea D'Amore)]

The error refers to par2fileformat.h:87 but that doesn't seems a variable length arrays, a vla should have an int variable as array size. In this case [] is used as pointer notation in the C sense, the memory itself is allocated in verificationpacket.cpp by function AllocatePacket(), whose definition I wasn't able to find, my cpp-fu is weak.

The fix I can see is just replacing entries[] with *entries as per attached patch, it succesfully built libpar2 on my Xcode 4.3 system. I tried port par2's test scripts as well, 3 out of 6 failed but I don't know if this is due to the upward fix. I'm leaving the review to someone who knows par2 or can actually test it on a real case.

This could be a clang issue if that syntax is recognized as variable length arrays while in fact it is not.

[jeremyhu (Jeremy Huddleston Sequoia)]

Yeah, with llvm-gcc it passes all 6 tests. With clang, it fails 4 out of 6 here...

[anddam (Andrea D'Amore)]

The problem there is about port par2, are the failing tests due to this specific issue or are they clang related but not to the fixed libpar2?

If the latter we should commit libpar2 and close this ticket.

I think the [] syntax in that struct is just a C pointer and the patch is fine, but I'd like to get a specific review about that. Anyone?

[neverpanic (Clemens Lang)]

JFYI, this is the reason the nzbget port is set to use llvm-gcc-4.2. It would be great if this was fixed, so I can drop the compiler change.

As to the question, yes type name[] is equivalent to type *name in this case as far as I know. Please commit the patch.

[neverpanic (Clemens Lang)]

Alternatively, one could probably use type name[0].

[jeremyhu (Jeremy Huddleston Sequoia)]

Replying to and.damore@…:

The problem there is about port par2, are the failing tests due to this specific issue or are they clang related but not to the fixed libpar2?

I don't know. There are no tests in libpar2, so I was using par2's tests to verify both since they share so much code.

[anddam (Andrea D'Amore)]

Replying to cal@…:

JFYI, this is the reason the nzbget port is set to use llvm-gcc-4.2. It would be great if this was fixed, so I can drop the compiler change. As to the question, yes type name[] is equivalent to type *name in this case as far as I know. Please commit the patch.

Committed r93338, ticket closed.

[jeremyhu (Jeremy Huddleston Sequoia)]

And reverted in r93350 ...

[jmroot (Joshua Root)]

Replying to cal@…:

Alternatively, one could probably use type name[0].

This is what FreeBSD does.

[anddam (Andrea D'Amore)]

Replying to jeremyhu@…:

And reverted in r93350 ...

Did you revert due to par2 failing tests?

[jeremyhu (Jeremy Huddleston Sequoia)]

Replying to and.damore@…:

Replying to jeremyhu@…:

And reverted in r93350 ...

Did you revert due to par2 failing tests?

Yes, as mentioned in the Portfile comment, the commit message, and this ticket ;)

[jmroot (Joshua Root)]

Presumably resolved by r141223 and r141225.