On 08/05/2018 04:28 AM, Bernd Edlinger wrote: > Hi, > > I would like to do a minor tweak to the patch. > While staring at the other patch I realized that I should > better pass size and not thissize to the check > function, instead of making use of how thissize is > computed using MIN above. This means that this condition > > + if ((unsigned)len != size && (unsigned)len != size + elts) > + return false; > > should better and more readable be: > > + if (size < (unsigned)len && size != len - elts) > + return false; > > Furthermore I wanted to have the check function static, > which I missed to do previously. > > For completeness, these are again the precondition patches > for this patch: > > [PATCH] Handle overlength strings in the C FE > https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00142.html I believe Joseph ack'd this already:
https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00655.html > > [PATCH] Make GO string literals properly NUL terminated > https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01931.html Ian didn't object, deferring to the larger scoped review. My understanding is this should be a nop for Go. Given it takes us closer to have a canonical form, I'll go ahead and ACK for the trunk. > > [PATCH] [Ada] Make middle-end string literals NUL terminated > https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01929.html This is OK. > > [PATCH] Create internally nul terminated string literals in fortan FE > https://gcc.gnu.org/ml/fortran/2018-08/msg00000.html This is OK too. THough it is unclear why we're not fixing gfc_build_string_const vs introducing gfc_build_hollerith_string_const. Are hollerith strings naturally not terminated in the fortran front-end and thus need special handling, while other calls to gfc_build_string_const are always with naturally nul terminated strings? > > > Bootstrapped, as usual. > Is it OK for trunk? So as a toplevel comment. While there may be some interaction with Martin's code to detect and warn for unterminated strings passed to strlen and other cases, I think the overall goal of canonicalizing our internal representation of STRING_CST along with a verification step is a good thing and it may make some of Martin's work easier. At this point are the prereqs of the varasm verification patch all ACK'd? Jeff
