From 851c19fa6b27178d1c68c664e6ff5950d4e65b85 Mon Sep 17 00:00:00 2001 From: Victor Robertson Date: Mon, 15 Aug 2016 15:19:59 -0700 Subject: [PATCH] Fix strip-eol for long command output Previously, when shell command output exceeded the 1024 character buffer size, each chunk of output would be stripped. This had the undesirable effect of sometimes breaking compilation by splitting on whitespace boundaries. This patch addresses the issue by providing a string_rtrim function and utilizing the function on the output string instead of each buffered component. --- src/engine/builtins.c | 18 +++++------------- src/engine/strings.c | 26 ++++++++++++++++++++++++++ src/engine/strings.h | 1 + test/core-language/test.jam | 25 +++++++++++++++++++++++-- 4 files changed, 55 insertions(+), 15 deletions(-) diff --git a/src/engine/builtins.c b/src/engine/builtins.c index 4bb6a62..18b2260 100644 --- a/src/engine/builtins.c +++ b/src/engine/builtins.c @@ -450,7 +450,7 @@ void load_builtins() char const * args [] = { "path", 0 }; bind_builtin( "MAKEDIR", builtin_makedir, 0, args ); } - + { const char * args [] = { "path", 0 }; bind_builtin( "READLINK", builtin_readlink, 0, args ); @@ -1981,7 +1981,7 @@ LIST *builtin_readlink( FRAME * frame, int flags ) bufsize *= 2; buf = BJAM_MALLOC( bufsize ); } - + if ( buf != static_buf ) BJAM_FREE( buf ); @@ -2460,15 +2460,6 @@ PyObject * bjam_caller( PyObject * self, PyObject * args ) #endif /* defined(_MSC_VER) || defined(__BORLANDC__) */ -static char * rtrim( char * const s ) -{ - char * p = s; - while ( *p ) ++p; - for ( --p; p >= s && isspace( *p ); *p-- = 0 ); - return s; -} - - LIST * builtin_shell( FRAME * frame, int flags ) { LIST * command = lol_get( frame->args, 0 ); @@ -2515,8 +2506,6 @@ LIST * builtin_shell( FRAME * frame, int flags ) buffer[ ret ] = 0; if ( !no_output_opt ) { - if ( strip_eol_opt ) - rtrim( buffer ); string_append( &s, buffer ); } @@ -2524,6 +2513,9 @@ LIST * builtin_shell( FRAME * frame, int flags ) if ( feof( p ) ) break; } + if ( strip_eol_opt ) + string_rtrim( &s ); + exit_status = pclose( p ); /* The command output is returned first. */ diff --git a/src/engine/strings.c b/src/engine/strings.c index 3d3e19b..19f03c9 100644 --- a/src/engine/strings.c +++ b/src/engine/strings.c @@ -189,6 +189,12 @@ char string_back( string * self ) return self->value[ self->size - 1 ]; } +void string_rtrim( string * self ) +{ + assert_invariants( self ); + char * p = self->value + self->size - 1; + for ( p; p >= self->value && ( *p == '\0' || isspace( *p ) ); *p-- = 0 ); +} #ifndef NDEBUG void string_unit_test() @@ -219,5 +225,25 @@ void string_unit_test() assert( copy->size == strlen( original ) ); string_free( copy ); } + + { + char * const foo = "Foo "; + string foo_copy[ 1 ]; + string_copy( foo_copy, foo ); + string_rtrim( foo_copy ); + assert( !strcmp( foo_copy->value, "Foo" ) ); + + string_rtrim( foo_copy ); + assert( !strcmp( foo_copy->value, "Foo" ) ); + + char * const bar = "Bar\0\0\0"; + string bar_copy[ 1 ]; + string_copy( bar_copy, bar ); + string_rtrim( bar_copy ); + assert( !strcmp( bar_copy->value, "Bar" ) ); + + string_rtrim( bar_copy ); + assert( !strcmp( bar_copy->value, "Bar" ) ); + } } #endif diff --git a/src/engine/strings.h b/src/engine/strings.h index 749f287..81a40cc 100644 --- a/src/engine/strings.h +++ b/src/engine/strings.h @@ -31,6 +31,7 @@ void string_reserve( string *, size_t ); void string_truncate( string *, size_t ); void string_pop_back( string * ); char string_back( string * ); +void string_rtrim( string * ); void string_unit_test(); #endif diff --git a/test/core-language/test.jam b/test/core-language/test.jam index b0ac767..63a5373 100644 --- a/test/core-language/test.jam +++ b/test/core-language/test.jam @@ -1239,7 +1239,7 @@ for y in $(values) } check-order break-for-exec : r1-v1 ; -check-equal break-for-cleanup : $(z) : original ; +check-equal break-for-cleanup : $(z) : original ; for local y in $(values) { @@ -1250,7 +1250,7 @@ for local y in $(values) } check-order break-for-local-exec : r1-v1 ; -check-equal break-for-local-cleanup : $(z) : original ; +check-equal break-for-local-cleanup : $(z) : original ; local z1 = z1val ; local z2 = z2val ; @@ -1505,11 +1505,32 @@ check-equal shell : "value\n" : [ SHELL $(c) ] ; check-equal shell : "" : [ SHELL $(c) : no-output ] ; check-equal shell : "value\n" 0 : [ SHELL $(c) : exit-status ] ; check-equal shell : "" 0 : [ SHELL $(c) : no-output : exit-status ] ; +check-equal shell : "" 0 : [ SHELL $(c) : no-output : exit-status : strip-eol ] ; check-equal command : "value\n" : [ COMMAND $(c) ] ; check-equal command : "" : [ COMMAND $(c) : no-output ] ; check-equal command : "value\n" 0 : [ COMMAND $(c) : exit-status ] ; check-equal command : "" 0 : [ COMMAND $(c) : no-output : exit-status ] ; +# buffered output + +local expected = "When the shell output buffer splits on whitespace, the whitespace shouldn't be trimmed. end." ; +local buffered = "echo \"$(expected)\"" ; +if $(OS) = VMS { buffered = "PIPE WRITE SYS$OUTPUT \"$(expected)\"" ; } + +check-equal shell : "$(expected)\n" : [ SHELL $(buffered) ] ; +check-equal shell : "" : [ SHELL $(buffered) : no-output ] ; +check-equal shell : "$(expected)\n" 0 : [ SHELL $(buffered) : exit-status ] ; +check-equal shell : "$(expected)" 0 : [ SHELL $(buffered) : strip-eol : exit-status ] ; +check-equal shell : "" 0 : [ SHELL $(buffered) : no-output : exit-status ] ; +check-equal shell : "" 0 : [ SHELL $(buffered) : no-output : exit-status : strip-eol ] ; +check-equal shell : "$(expected)" 0 : [ SHELL $(buffered) : strip-eol : exit-status ] ; + +check-equal command : "$(expected)\n" : [ COMMAND $(buffered) ] ; +check-equal command : "" : [ COMMAND $(buffered) : no-output ] ; +check-equal command : "$(expected)\n" 0 : [ COMMAND $(buffered) : exit-status ] ; +check-equal command : "$(expected)" 0 : [ COMMAND $(buffered) : strip-eol : exit-status ] ; +check-equal command : "" 0 : [ COMMAND $(buffered) : no-output : exit-status ] ; + } # Test SUBST -- 2.10.1 (Apple Git-78)