>> For me, it's not clear why you add elog(ERROR), rather than Assert. If >> the infinite loop should not happen when our program is correct, it >> would be better to fix the problem and use Assert. >> > > Assert will be optimized out in released builds.
Sure. > Using elog(ERROR) can catch such unlikely errors in release builds. On the hand if we are confident that the error is truley unlikely happen, using elog(ERROR) is just a waste of CPU. > Given how recursive the whole patchset is, even with 100% test coverage, I > think > it would still be better to use elog(ERROR) to guard against some code > paths that should not be reached. Ok. You are not confident the errors never happen. If so, using elog(ERROR) makes sense. > I also intend to add below at the beginning of nfa_state_free > `````` > /* state at the free-list head was freed by the previous call */ > if (unlikely(winstate->nfaStateFree == state)) > elog(ERROR, "double free of RPR NFA state"); > `````` Looks good to me. Probably we need more elog(ERROR) in nfa modules? Regards, -- Tatsuo Ishii SRA OSS K.K. English: http://www.sraoss.co.jp/index_en/ Japanese:http://www.sraoss.co.jp
