accepted unacceptable domain

[massimomorara]

Parsing the following xml

<?xml version="1.0" encoding="UTF-8"?>
<instance format="XCSP3" type="CSP">
  <variables>
    <var id="x">foo..bar</var>
  </variables>
</instance>

the (unacceptable, I hope) interval foo..bar is accepted as 0..0 (but the exact values are undefined, according the C++ standard).

The problem is in XMLParser::parseDomain() (XMLParser.cc) where is used the method to() of UTF8String to detect the first and the last value of the interval

token.substr(0, pos).to(first);
token.substr(pos + 2).to(last);

but there isn't a check for the value returned by to() that say if there was an error in the detection.

Same problem with the single value part of the function: the is no check for the value returned by

token.to(val);

so is accepted a variable as

<var id="x">foo</var>

A possible solution for the problem is the following

void XMLParser::parseDomain(const UTF8String txt, XDomainInteger *domain) {
    static std::string const preErr { "error parsing domain \"" };
    static UTF8String const dotdot { ".." };
    UTF8String::Tokenizer tokenizer { txt };
    while(tokenizer.hasMoreTokens()) {
        UTF8String token = tokenizer.nextToken();
        size_t pos = token.find(dotdot);

        if(pos == UTF8String::npos) {
            int val;

            if ( false == token.to(val) ) {
                std::string ds;
                txt.to(ds);
                throw std::runtime_error { preErr + ds + "\"" };
            }

            domain->addValue(val);
        } else {
            int first, last;

            if (    (false == token.substr(0, pos).to(first))
                 || (false == token.substr(pos + 2).to(last)) ) {
                std::string ds;
                txt.to(ds);
                throw std::runtime_error { preErr + ds + "\"" };
            }

            domain->addInterval(first, last);
        }
    }
}

but I suppose that the problem is in other method too (in the following void XMLParser::parseListOfIntegerOrInterval(), by example)

En passant... in parseDomain() (as similarly in other methods/functions) you pass txt as const UTF8String and domain as pointer to XDomainInteger.

I suggest to pass txt as a const reference (UTF8String const & txt), to avoid an unuseful copy of the object, and domain as a reference to XDomainInteger (XDomainInteger & domain) to avoid the risk that parseDomain() is called with an unchecked null pointer.

[xcsp3team]

This is done. Thanks. I need to check if this appear elsewhere.

[massimomorara]

Good. There is just a misprint in parseDomain(): "Intger" (two times) that should be "Integer".

[xcsp3team]

Note that foo..bar should be foo..bar

The type attribute is required for all types of variables except for integer variables.

[massimomorara]

Sorry but I don't understand the part "should be foo..bar"

[xcsp3team]

Sorry, one part of my answer was missing. I just wanted to say that if you want to declare a symbolic variable, it is required to have type="symbolic", as for example : foo Concerning foo..bar this seems incorrect to me because it is not possible to define intervals with symbols.

[massimomorara]

I did not explain well (my English is very poor; sorry).

I try with other word.

My intention, opening this issue was to show that, if you define a integer variable using garbage, there wasn't a valid check to detect that there is a correct number. Or better: there was the check in the function used to extract the number but the returned value was unused.

In other words: foo and bar wasn't intended as symbolic variable but only as sequences of letters where should be numbers.