[apparmor] [patch 19/24] Move buffer management for the interface to C++ ostringstream class
John Johansen
john.johansen at canonical.com
Fri Mar 14 22:36:35 UTC 2014
On 03/14/2014 01:47 PM, Steve Beattie wrote:
> On Fri, Mar 07, 2014 at 09:31:40AM -0800, john.johansen at canonical.com wrote:
>> Signed-off-by: John Johansen <john.johansen at canonical.com>
>> ---
>> parser/parser.h | 5
>> parser/parser_interface.c | 510 +++++++++++++---------------------------------
>> parser/parser_policy.c | 8
>> 3 files changed, 151 insertions(+), 372 deletions(-)
>
> When compiling on 32 bit platforms after applying this patch, the
> compiler vomits up the following warnings:
>
> g++ -g -O2 -pipe -Wall -Wsign-compare -Wmissing-field-initializers -Wformat-security -Wunused-parameter -std=gnu++0x -D_GNU_SOURCE -DPACKAGE=\"apparmor-parser\" -DLOCALEDIR=\"/usr/share/locale\" -DSUBDOMAIN_CONFDIR=\"/etc/apparmor\" -I../libraries/libapparmor//include -c -o parser_interface.o parser_interface.c
> parser_interface.c: In function ‘void sd_write_aligned_blob(std::ostringstream&, void*, int, const char*)’:
> parser_interface.c:275:37: warning: ISO C++ says that these are ambiguous, even though the worst conversion for the first is better than the worst conversion for the second: [enabled by default]
> size_t pad = align64(buf.tellp() + 5l) - (buf.tellp() + 5l);
> ^
> parser_interface.c:269:23: note: in definition of macro ‘align64’
> #define align64(X) (((X) + (typeof(X)) 7) & ~((typeof(X)) 7))
> ^
> In file included from /usr/include/c++/4.8/bits/char_traits.h:40:0,
> from /usr/include/c++/4.8/string:40,
> from parser_interface.c:30:
> /usr/include/c++/4.8/bits/postypes.h:178:7: note: candidate 1: std::fpos<_StateT> std::fpos<_StateT>::operator+(std::streamoff) const [with _StateT = __mbstate_t; std::streamoff = long long int]
> operator+(streamoff __off) const
> ^
> parser_interface.c:275:37: note: candidate 2: operator+(std::streamoff {aka long long int}, long int) <built-in>
> size_t pad = align64(buf.tellp() + 5l) - (buf.tellp() + 5l);
> ^
> parser_interface.c:269:23: note: in definition of macro ‘align64’
> #define align64(X) (((X) + (typeof(X)) 7) & ~((typeof(X)) 7))
> ^
> parser_interface.c:275:37: warning: ISO C++ says that these are ambiguous, even though the worst conversion for the first is better than the worst conversion for the second: [enabled by default]
> size_t pad = align64(buf.tellp() + 5l) - (buf.tellp() + 5l);
> ^
> parser_interface.c:269:36: note: in definition of macro ‘align64’
> #define align64(X) (((X) + (typeof(X)) 7) & ~((typeof(X)) 7))
> ^
> In file included from /usr/include/c++/4.8/bits/char_traits.h:40:0,
> from /usr/include/c++/4.8/string:40,
> from parser_interface.c:30:
> /usr/include/c++/4.8/bits/postypes.h:178:7: note: candidate 1: std::fpos<_StateT> std::fpos<_StateT>::operator+(std::streamoff) const [with _StateT = __mbstate_t; std::streamoff = long long int]
> operator+(streamoff __off) const
> ^
> parser_interface.c:275:37: note: candidate 2: operator+(std::streamoff {aka long long int}, long int) <built-in>
> size_t pad = align64(buf.tellp() + 5l) - (buf.tellp() + 5l);
> ^
> parser_interface.c:269:36: note: in definition of macro ‘align64’
> #define align64(X) (((X) + (typeof(X)) 7) & ~((typeof(X)) 7))
> ^
> parser_interface.c:275:37: warning: ISO C++ says that these are ambiguous, even though the worst conversion for the first is better than the worst conversion for the second: [enabled by default]
> size_t pad = align64(buf.tellp() + 5l) - (buf.tellp() + 5l);
> ^
> parser_interface.c:269:55: note: in definition of macro ‘align64’
> #define align64(X) (((X) + (typeof(X)) 7) & ~((typeof(X)) 7))
> ^
> In file included from /usr/include/c++/4.8/bits/char_traits.h:40:0,
> from /usr/include/c++/4.8/string:40,
> from parser_interface.c:30:
> /usr/include/c++/4.8/bits/postypes.h:178:7: note: candidate 1: std::fpos<_StateT> std::fpos<_StateT>::operator+(std::streamoff) const [with _StateT = __mbstate_t; std::streamoff = long long int]
> operator+(streamoff __off) const
> ^
> parser_interface.c:275:37: note: candidate 2: operator+(std::streamoff {aka long long int}, long int) <built-in>
> size_t pad = align64(buf.tellp() + 5l) - (buf.tellp() + 5l);
> ^
> parser_interface.c:269:55: note: in definition of macro ‘align64’
> #define align64(X) (((X) + (typeof(X)) 7) & ~((typeof(X)) 7))
> ^
> parser_interface.c:275:58: warning: ISO C++ says that these are ambiguous, even though the worst conversion for the first is better than the worst conversion for the second: [enabled by default]
> size_t pad = align64(buf.tellp() + 5l) - (buf.tellp() + 5l);
> ^
> In file included from /usr/include/c++/4.8/bits/char_traits.h:40:0,
> from /usr/include/c++/4.8/string:40,
> from parser_interface.c:30:
> /usr/include/c++/4.8/bits/postypes.h:178:7: note: candidate 1: std::fpos<_StateT> std::fpos<_StateT>::operator+(std::streamoff) const [with _StateT = __mbstate_t; std::streamoff = long long int]
> operator+(streamoff __off) const
> ^
> parser_interface.c:275:58: note: candidate 2: operator+(std::streamoff {aka long long int}, long int) <built-in>
> size_t pad = align64(buf.tellp() + 5l) - (buf.tellp() + 5l);
> ^
>
> I freely admit that I don't quite get what's going wrong here, except
> that there's some ambiguity between the type returned by
> std::ostringstream.tellp() (which is std::streamoff) and 5l. The
> following silences the warnings (and doesn't produce them on 64 bit
> platforms), but I'm not confident it's the right solution.
>
Its because there are 2 candidates when resolving the overloaded operator
+ mixed with C++ rules around constant typing are just stupid. basically
there is
fpos +(streamoff, long long int)
long long +(streamoff, long int)
the compiler is trying to resolve a criss cross of the return type and the
second parameter. It thinks the second fn matches better for the argument
but not the return type.
the other way of resolving this would be changing the type of size_t pad.
Overall I think the cast is more explicit so it is probably the best
way to go
> ---
> parser/parser_interface.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Index: b/parser/parser_interface.c
> ===================================================================
> --- a/parser/parser_interface.c
> +++ b/parser/parser_interface.c
> @@ -269,7 +269,7 @@ inline void sd_write_aligned_blob(std::o
> {
> sd_write_name(buf, name);
> /* pad calculation MUST come after name is written */
> - size_t pad = align64(buf.tellp() + 5l) - (buf.tellp() + 5l);
> + size_t pad = align64(buf.tellp() + ((std::streamoff) 5l)) - (buf.tellp() + ((std::streamoff) 5l));
> sd_write8(buf, SD_BLOB);
> sd_write32(buf, b_size + pad);
> buf.write(zeros, pad);
>
>
>
>
More information about the AppArmor
mailing list