[Asterisk-code-review] Update configure.ac/Makefile for clang (asterisk[13])

[Corey Farrell]

Corey Farrell has posted comments on this change.

Change subject: Update configure.ac/Makefile for clang
......................................................................


Patch Set 7: Code-Review-1

(3 comments)

> If we need to change the detection method as proposed by you, i
 > will do so.

I would prefer this, but won't block the change for it.  Long term I think it's best to always detect features, and stay ignorant about the compiler being used as much as possible.

https://gerrit.asterisk.org/#/c/157/7/autoconf/ast_check_raii.m4
File autoconf/ast_check_raii.m4:

Line 33: 			AC_MSG_CHECKING(for gcc -fnested-functions)
Looks like a copy/paste error, should have the check for clang/fblocks here.


https://gerrit.asterisk.org/#/c/157/7/autoconf/ast_check_strsep_array_bounds.m4
File autoconf/ast_check_strsep_array_bounds.m4:

Line 43: 					const char *teststr2 = "test,test";
I know this is just a configure/test program, but lets still follow the coding guideline about keeping variable declarations at the top of the block.

It seems like you could get rid of teststr2 by testing strcmp first, then strsep (using teststr1 for both).


Line 56: 			AC_DEFINE([_HAVE_STRING_ARCH_strcmp], 1, [Prevent clang array-bounds warning when using of bits/string2.h version of strcmp])
A comment about how this is effective would be nice.  I looked at the LLVM bug you linked and didn't find any information about this (or any other) work-around.


-- 
To view, visit https://gerrit.asterisk.org/157
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I12ea29d3bda2254ad3908e279b7effbbac6a97cb
Gerrit-PatchSet: 7
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Diederik de Groot <dkgroot at talon.nl>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Diederik de Groot <dkgroot at talon.nl>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: Yes