diff --git a/ChangeLog b/ChangeLog index 55ba1ce99..0aa3fe0b3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,11 @@ -2004-06-01 Alexandre Duret-Lutz +2004-06-02 Alexandre Duret-Lutz + + * HACKING, src/sanity/style.test: NULL is not portable, prohibit it. + * iface/gspn/ssp.cc, src/ltltest/reduc.cc, src/ltltest/syntimpl.cc, + src/ltlvisit/basereduc.cc, src/ltlvisit/reducform.cc, + src/ltlvisit/syntimpl.cc: Use 0 instead of NULL. + +2004-06-01 Alexandre Duret-Lutz * src/ltltest/inf.cc, src/ltltest/inf.test: Rename as ... * src/ltltest/syntimpl.cc, src/ltltest/syntimpl.test: ... these. diff --git a/HACKING b/HACKING index ac37ee49c..74bc6e70f 100644 --- a/HACKING +++ b/HACKING @@ -172,6 +172,25 @@ Formating } } + * Pointers and references are part of the type, and should be put + near the type, not near the variable. + + int* p; // not `int *p;' + list& l; // not `list &l;' + void* magic(); // not `void *magic();' + + * Do not declare many variables on one line. + Use + int* p; + int* q; + instead of + int *p, *q; + The former declarations also allow you to describe each variable. + + * The include guard for src/somedir/foo.hh is + SPOT_SOMEDIR_FOO_HH + + Naming ====== @@ -212,20 +231,15 @@ Naming * C Macros are all uppercase. - * Pointers and references are part of the type, and should be put - near the type, not near the variable. - int* p; // not `int *p;' - list& l; // not `list &l;' - void* magic(); // not `void *magic();' +Other style recommandations +=========================== - * Do not declare many variables on one line. - Use - int* p; - int* q; - instead of - int *p, *q; - The former declarations also allow you to describe each variable. + * Do not use the NULL macro, it is not always implemented in a way + which is compatible with all pointer types. Always use 0 instead. - * The include guard for src/somedir/foo.hh is - SPOT_SOMEDIR_FOO_HH + * Limit the scope of local variables by defining them as late as + possible. Do not reuse a local variables for two different things. + + * Do not systematically initialise local variables with 0 or other + meaningless values. This hides errors to valgrind. diff --git a/iface/gspn/ssp.cc b/iface/gspn/ssp.cc index 1943afa87..c4f42d0d4 100644 --- a/iface/gspn/ssp.cc +++ b/iface/gspn/ssp.cc @@ -938,7 +938,7 @@ namespace spot { if (spot_inclusion(old_state->left(), new_state->left())) { - State* succ_tgba_ = NULL; + State* succ_tgba_ = 0; size_t size_tgba_ = 0; succ_queue& queue = todo.top().second; diff --git a/src/ltltest/reduc.cc b/src/ltltest/reduc.cc index e003ebb5b..a81ab6972 100644 --- a/src/ltltest/reduc.cc +++ b/src/ltltest/reduc.cc @@ -77,7 +77,7 @@ main(int argc, char** argv) spot::ltl::parse_error_list p1; spot::ltl::formula* f1 = spot::ltl::parse(argv[2], p1); - spot::ltl::formula* f2 = NULL; + spot::ltl::formula* f2 = 0; if (spot::ltl::format_parse_errors(std::cerr, argv[2], p1)) return 2; @@ -119,7 +119,7 @@ main(int argc, char** argv) bool red = (length_f1_after < length_f1_before); std::string f2s = ""; - if (f2 != NULL) + if (f2) { ftmp1 = f2; f2 = unabbreviate_logic(f2); @@ -170,7 +170,7 @@ main(int argc, char** argv) } spot::ltl::destroy(f1); - if (f2 != NULL) + if (f2) spot::ltl::destroy(f2); diff --git a/src/ltltest/syntimpl.cc b/src/ltltest/syntimpl.cc index ff5ba4ab8..b51e78e3a 100644 --- a/src/ltltest/syntimpl.cc +++ b/src/ltltest/syntimpl.cc @@ -47,13 +47,12 @@ main(int argc, char** argv) spot::ltl::parse_error_list p1; spot::ltl::formula* f1 = spot::ltl::parse(argv[2], p1); - spot::ltl::formula* f2 = NULL; if (spot::ltl::format_parse_errors(std::cerr, argv[2], p1)) return 2; spot::ltl::parse_error_list p2; - f2 = spot::ltl::parse(argv[3], p2); + spot::ltl::formula* f2 = spot::ltl::parse(argv[3], p2); if (spot::ltl::format_parse_errors(std::cerr, argv[3], p2)) return 2; diff --git a/src/ltlvisit/basereduc.cc b/src/ltlvisit/basereduc.cc index c80b89f60..32d479064 100644 --- a/src/ltlvisit/basereduc.cc +++ b/src/ltlvisit/basereduc.cc @@ -90,9 +90,9 @@ namespace spot { formula* f = uo->child(); result_ = basic_reduce(f); - multop* mo = NULL; - unop* u = NULL; - binop* bo = NULL; + multop* mo = 0; + unop* u = 0; + binop* bo = 0; switch (uo->op()) { case unop::Not: @@ -261,9 +261,8 @@ namespace spot { formula* f1 = bo->first(); formula* f2 = bo->second(); - formula* ftmp = NULL; - unop* fu1 = NULL; - unop* fu2 = NULL; + unop* fu1; + unop* fu2; switch (bo->op()) { case binop::Xor: @@ -301,9 +300,9 @@ namespace spot (fu1->op() == unop::X) && (fu2->op() == unop::X)) { - ftmp = binop::instance(binop::U, - basic_reduce(fu1->child()), - basic_reduce(fu2->child())); + formula* ftmp = binop::instance(binop::U, + basic_reduce(fu1->child()), + basic_reduce(fu2->child())); result_ = unop::instance(unop::X, basic_reduce(ftmp)); destroy(f1); destroy(f2); @@ -334,9 +333,9 @@ namespace spot (fu1->op() == unop::X) && (fu2->op() == unop::X)) { - ftmp = binop::instance(bo->op(), - basic_reduce(fu1->child()), - basic_reduce(fu2->child())); + formula* ftmp = binop::instance(bo->op(), + basic_reduce(fu1->child()), + basic_reduce(fu2->child())); result_ = unop::instance(unop::X, basic_reduce(ftmp)); destroy(f1); destroy(f2); @@ -361,27 +360,23 @@ namespace spot multop::vec* tmpX = new multop::vec; multop::vec* tmpU = new multop::vec; - multop::vec* tmpUright = NULL; multop::vec* tmpR = new multop::vec; - multop::vec* tmpRright = NULL; multop::vec* tmpFG = new multop::vec; multop::vec* tmpGF = new multop::vec; multop::vec* tmpOther = new multop::vec; - formula* ftmp = NULL; - for (unsigned i = 0; i < mos; ++i) res->push_back(basic_reduce(mo->nth(i))); - switch (op) { case multop::And: for (multop::vec::iterator i = res->begin(); i != res->end(); i++) { - if (*i == NULL) + // FIXME: why would *i be 0 ? + if (!*i) continue; unop* uo = dynamic_cast(*i); binop* bo = dynamic_cast(*i); @@ -408,8 +403,8 @@ namespace spot if (bo->op() == binop::U) { // (a U b) & (c U b) = (a & c) U b - ftmp = dynamic_cast(*i)->second(); - tmpUright = new multop::vec; + formula* ftmp = dynamic_cast(*i)->second(); + multop::vec* tmpUright = new multop::vec; for (multop::vec::iterator j = i; j != res->end(); j++) { if (!*j) @@ -438,8 +433,8 @@ namespace spot else if (bo->op() == binop::R) { // (a R b) & (a R c) = a R (b & c) - ftmp = dynamic_cast(*i)->first(); - tmpRright = new multop::vec; + formula* ftmp = dynamic_cast(*i)->first(); + multop::vec* tmpRright = new multop::vec; for (multop::vec::iterator j = i; j != res->end(); j++) { if (!*j) @@ -512,8 +507,8 @@ namespace spot if (bo->op() == binop::U) { // (a U b) | (a U c) = a U (b | c) - ftmp = bo->first(); - tmpUright = new multop::vec; + formula* ftmp = bo->first(); + multop::vec* tmpUright = new multop::vec; for (multop::vec::iterator j = i; j != res->end(); j++) { if (!*j) @@ -540,8 +535,8 @@ namespace spot else if (bo->op() == binop::R) { // (a R b) | (c R b) = (a | c) R b - ftmp = dynamic_cast(*i)->second(); - tmpRright = new multop::vec; + formula* ftmp = dynamic_cast(*i)->second(); + multop::vec* tmpRright = new multop::vec; for (multop::vec::iterator j = i; j != res->end(); j++) { if (!*j) @@ -604,22 +599,22 @@ namespace spot else delete tmpR; - if ((tmpGF != NULL) && - (tmpGF->size())) + if (tmpGF && tmpGF->size()) { - ftmp = unop::instance(unop::G, - unop::instance(unop::F, - multop::instance(mo->op(), - tmpGF))); + formula* ftmp + = unop::instance(unop::G, + unop::instance(unop::F, + multop::instance(mo->op(), + tmpGF))); tmpOther->push_back(ftmp); } - if ((tmpFG != NULL) && - (tmpFG->size())) + if (tmpFG && tmpFG->size()) { - ftmp = unop::instance(unop::F, - unop::instance(unop::G, - multop::instance(mo->op(), - tmpFG))); + formula* ftmp + = unop::instance(unop::F, + unop::instance(unop::G, + multop::instance(mo->op(), + tmpFG))); tmpOther->push_back(ftmp); } diff --git a/src/ltlvisit/reducform.cc b/src/ltlvisit/reducform.cc index 300eaf685..bac3aa0c6 100644 --- a/src/ltlvisit/reducform.cc +++ b/src/ltlvisit/reducform.cc @@ -193,28 +193,24 @@ namespace spot void visit(multop* mo) { - formula* f1 = NULL; - formula* f2 = NULL; unsigned mos = mo->size(); multop::vec* res = new multop::vec; - multop::vec::iterator index; - multop::vec::iterator indextmp; for (unsigned i = 0; i < mos; ++i) res->push_back(recurse(mo->nth(i))); if (opt_ & Reduce_Syntactic_Implications) { + formula* f1; + formula* f2; + multop::vec::iterator index = res->begin(); + multop::vec::iterator indextmp = index; switch (mo->op()) { - case multop::Or: - index = indextmp = res->begin(); if (index != res->end()) - { - f1 = *index; - index++; - } + break; + f1 = *index++; for (; index != res->end(); index++) { f2 = *index; @@ -237,12 +233,9 @@ namespace spot break; case multop::And: - index = indextmp = res->begin(); - if (mos) - { - f1 = mo->nth(0); - index++; - } + if (index != res->end()) + break; + f1 = *index++; for (; index != res->end(); index++) { f2 = *index; diff --git a/src/ltlvisit/syntimpl.cc b/src/ltlvisit/syntimpl.cc index 06c4c5312..7d9becb32 100644 --- a/src/ltlvisit/syntimpl.cc +++ b/src/ltlvisit/syntimpl.cc @@ -220,7 +220,6 @@ namespace spot visit(const unop* uo) { const formula* f1 = uo->child(); - const formula* tmp = NULL; switch (uo->op()) { case unop::Not: @@ -240,10 +239,8 @@ namespace spot return; case unop::G: /* G(a) = false R a */ - tmp = constant::false_instance(); - if (syntactic_implication(f, tmp)) + if (syntactic_implication(f, constant::false_instance())) result_ = true; - destroy(tmp); return; } /* Unreachable code. */ diff --git a/src/sanity/style.test b/src/sanity/style.test index 2e20410ec..90114a699 100755 --- a/src/sanity/style.test +++ b/src/sanity/style.test @@ -113,6 +113,9 @@ for dir in "${INCDIR-..}" "${INCDIR-..}"/../iface; do grep -e 'return[ ]*[(][^(]*[)];' $tmp && diag 'No useless parentheses after delete.' + grep 'NULL' $tmp && + diag 'Use 0 instead of NULL. NULL is not portable.' + $fail && echo "$file" >>failures done done