Прокоментируйте код - C/C++

E>Если он правильно работает, то лучше его не трогать, пусть работает.
E>Если он работает не правильно, то лучше его выбросить и переписать все начисто.

Внимание вопрос! Как понять что он работает правильно и не бомбит память иногда

... << RSDN@Home 1.1.4 beta 4 rev. 303>>

Здравствуйте, Tom, Вы писали:

[]
Ещё подумалось: возможно yacc/bizon справился бы лучше

... << RSDN@Home 1.1.4 stable rev. 510>>

Здравствуйте, Tom, Вы писали:

E>>Если он правильно работает, то лучше его не трогать, пусть работает.
E>>Если он работает не правильно, то лучше его выбросить и переписать все начисто.

Tom>Внимание вопрос! Как понять что он работает правильно

unit-тестами?

Tom> и не бомбит память иногда

Это действительно вопрос. А что, есть подозрение?

... << RSDN@Home 1.1.4 stable rev. 510>>

Здравствуйте, Tom, Вы писали:

E>>Если он правильно работает, то лучше его не трогать, пусть работает.
E>>Если он работает не правильно, то лучше его выбросить и переписать все начисто.

Tom>Внимание вопрос! Как понять что он работает правильно и не бомбит память иногда

Purify, etc.

... << RSDN@Home 1.1.4 stable rev. 510>>

Здравствуйте, MaximE, Вы писали:

ME>Для alloca() не нужен free().

А точно...
100 лет уже не пользовал alloca

Здравствуйте, Tom, Вы писали:

E>>Если он правильно работает, то лучше его не трогать, пусть работает.
E>>Если он работает не правильно, то лучше его выбросить и переписать все начисто.

Tom>Внимание вопрос! Как понять что он работает правильно и не бомбит память иногда

Иногда железно бомбит хотя бы из-за alloca в цикле.
В общем если верно подобрать входные данные, то довольно легко все завалить.

Здравствуйте, srggal, Вы писали:

S>Ещё подумалось: возможно yacc/bizon справился бы лучше

из пушки по пернатым друзьям, ИМХО. строку вида name1=val1&name2=val2&name3=%2А можно самому распарсить кодом попроще, чем в оригинале.

... << RSDN@Home 1.2.0 alpha rev. 619>>

Здравствуйте, Глеб Алексеев, Вы писали:

S>>Ещё подумалось: возможно yacc/bizon справился бы лучше

ГА>из пушки по пернатым друзьям, ИМХО. строку вида name1=val1&name2=val2&name3=%2А можно самому распарсить кодом попроще, чем в оригинале.

Ну, не знаю. Я так делал:

//
// param_map_t
//

//! Словарь значений CGI-параметров.
typedef std::map< std::string, std::string >
    param_map_t;

#define HEXC2INT(c) ((c >= 'A' ? ( c - 'A' ) : ( c - '0' )))

// Преобразовать URL-кодировку в ASCII.
static void
url_decode( std::string & what )
{
    std::string::size_type i = 0;
    while( i != what.length() ) {
        char c = what[ i ];
        if( '+' == c )
            what.replace( i, 1, 1, ' ' );
        else if( '%' == c ) {
            // Расшифровать HEX представление.
            if( i + 2 < what.length() ) {
                char h1 = toupper( what[ i + 1 ] );
                char h2 = toupper( what[ i + 2 ] );
                if( isxdigit( h1 ) && isxdigit( h2 ) ) {
                    // Можно создавать ASCII представление.
                    char r = ( HEXC2INT( h1 ) << 4 ) |
                        HEXC2INT( h2 );
                    what.replace( i, 3, 1, r );
                    // Переменную i не изменяем, т.к. она
                    // указывает на замененный символ.
                }
            }
        }
        ++i;
    }
}

// Выделить из запроса все пары <name=value>.
static void
extract_all_pairs(
    const char * query_string,
    param_map_t & result )
{
    size_t start = 0, cur = 0;

    // Извлекаем имя параметра.
    while( query_string[ cur ] ) {
        std::string param_name;
        std::string param_value;

        if( '=' == query_string[ cur ] ) {
            // Имя параметра закончилось.
            // Имя параметра должно содержать хотя бы один символ.
            if( start != cur ) {
                param_name.assign( &query_string[ start ],
                    cur - start );
                // Пропускаем '='
                ++cur;
                start = cur;
                // Извлекаем значение параметра.
                while( query_string[ cur ] ) {
                    if( '&' == query_string[ cur ] )
                        break;
                    ++cur;
                }
                // Значение параметра закончилось.
                // Значение может быть пустым.
                if( start != cur ) {
                    param_value.assign( &query_string[ start ],
                        cur - start );
                }

                // Для пропуска '&'
                if( '&' == query_string[ cur ] )
                    start = cur + 1;

                // Извлекли и имя, и значение параметра.
                // Изменим кодировку.
                url_decode( param_name );
                url_decode( param_value );

                result.insert( param_map_t::value_type(
                    param_name, param_value ) );
            }
        }
        ++cur;
    }

}

//
// parse_query_string
//

/*
    Процедура разбора значения переменной среды QUERY_STRING
    на множество пар <имя параметра, значение параметра>.
*/
void
parse_query_string( param_map_t & result )
{
    const char * env_value = getenv( "QUERY_STRING" );
    if( env_value ) {
        // Можно осуществлять разбор.

        // Выделяем все пары значений.
        extract_all_pairs( env_value, result );
    }
}

Может и здесь какие-нибудь баги найдут.

... << RSDN@Home 1.1.4 stable rev. 510>>

Здравствуйте, Tom, Вы писали:

Tom>Принимаются любые коментарии к след. коду. Просто что вы думаете взглянув на него
Про выделение памяти на стеке в цикле я уж молчу. И так все понятно.
Ничего сложного в коде нет, но дизайн ........... Такие простые вещи минут за 10 минут ну хоть до такого оптимизируются.

void CGIParamsBase::Parse(const char *cstr, int iLen)
{
    //-----------------------------------------
    char* pNameBegin = const_cast<char*>(cstr);
    char* pNameEnd   = NULL;

    if (*pNameBegin == '&') pNameBegin++;

    pNameEnd = strtok(pNameBegin, "=&");

    if(pNameEnd == NULL) return;

    //-----------------------------------------
    char* pValueBegin = pNameEnd + 1;
    char* pValueEnd   = NULL;
    
    switch(*pNameEnd)
    {
    case '=':
        pValueEnd = strtok(pValueBegin, "&");
        if (pNameEnd - pNameBegin > 2)
                {
                    //.......................
                    ProcessParam(pNameBegin, pNameEnd - pNameBegin, pConverted, iNewLen);
        }
        
        if(pValueEnd == NULL) break;

    case '&':
        Parse(pNameEnd, strlen(pNameEnd));
    }
    
    return;
}

Принимаю укоры лишь по оптимизации дизайна, поскольку писагно на колене за 5 минут в UltraEdit и без проверки.

Tom>

Tom>void CGIParamsBase::Parse(const char *cstr, int iLen)
Tom>{
Tom>    char *str = const_cast<char*>(cstr);
Tom>    char *pNameBegin = str, *pNameEnd, *pValueBegin, *pValueEnd;
Tom>    while (pNameBegin - str < iLen)
Tom>    {
Tom>        if (*pNameBegin == '&')
Tom>            pNameBegin++;
Tom>        pNameEnd = pNameBegin;
Tom>        while (pNameEnd - str < iLen && *pNameEnd != '=' && *pNameEnd != '&')
Tom>            pNameEnd++;
Tom>        if (pNameEnd - str == iLen)
Tom>            break;
Tom>        if (*pNameEnd == '&')
Tom>        {
Tom>            pNameBegin = pNameEnd + 1;
Tom>            continue;
Tom>        }

Tom>        pValueEnd = pValueBegin = pNameEnd + 1;
Tom>        while (pValueEnd - str < iLen && *pValueEnd != '&')
Tom>            pValueEnd++;

Tom>        if (pNameEnd - pNameBegin > 2)
Tom>        {
Tom>            int iSrcLen = pValueEnd - pValueBegin;
Tom>            int iNewLen = 0;
Tom>            const char *pSrc = pValueBegin;
Tom>            char *pConverted = (char *)_alloca(iSrcLen);
Tom>            char *pDest = pConverted;
Tom>            for (; iSrcLen-- > 0; iNewLen++, pDest++, pSrc++)
Tom>            {
Tom>                if ((BYTE)*pSrc <= 0x1f || (BYTE)*pSrc >= 0x7f)
Tom>                    goto Cont;
Tom>                if (*pSrc == '+')
Tom>                    *pDest = ' ';
Tom>                else
Tom>                    if (*pSrc == '%')
Tom>                    {
Tom>                        *pDest = (char)(C2D(pSrc[1]) * 16 + C2D(pSrc[2]));
Tom>                        pSrc += 2;
Tom>                        iSrcLen -= 2;
Tom>                    }
Tom>                    else
Tom>                        *pDest = *pSrc;
Tom>            }
Tom>            ProcessParam(pNameBegin, pNameEnd - pNameBegin, pConverted, iNewLen);
Tom>        }
Tom>Cont:
Tom>        pNameBegin = pValueEnd;
Tom>    }
Tom>}
Tom>

Здравствуйте, srggal, Вы писали:

S>Ещё подумалось: возможно yacc/bizon справился бы лучше

Возможно, что это даже он

Здравствуйте, Alexmoon, Вы писали:

A>Принимаю укоры лишь по оптимизации дизайна, поскольку писагно на колене за 5 минут в UltraEdit и без проверки.
Укор №1 Вызовом strtok ты модифицируешь строку, переданную как const char*. Нехорошо.

... << RSDN@Home 1.2.0 alpha rev. 619>>

Здравствуйте, eao197, Вы писали:

[]

ИМХО на примере двух вариантов кода отдаю предпочтение yacc'у

... << RSDN@Home 1.1.4 stable rev. 510>>

Здравствуйте, eao197, Вы писали:

[]

E>

E>#define HEXC2INT(c) ((c >= 'A' ? ( c - 'A' ) : ( c - '0' )))
E>

Я пользовался таким кодом:

inline uint8_t hex2bin(unsigned char h, unsigned char l)
{
    h |= 0x20;
    h -= 0x30;
    h -= -(h > 9) & 0x27;
    l |= 0x20;
    l -= 0x30;
    l -= -(l > 9) & 0x27;
    return h << 4 | l;
}

Специально заоптимизирован чтобы не было условных переходов в сгенеренном коде.

Здравствуйте, Глеб Алексеев, Вы писали:

ГА>Здравствуйте, Alexmoon, Вы писали:

A>>Принимаю укоры лишь по оптимизации дизайна, поскольку писагно на колене за 5 минут в UltraEdit и без проверки.
ГА>Укор №1 Вызовом strtok ты модифицируешь строку, переданную как const char*. Нехорошо.

Спасибо.

Может и не прилично с моей стороны оправдываться, но не урок а поправка внимательности. Такие вещи как то что не модифицируемые строки передаются по константному указателю, если уж быть совсем корректными, нужно отслеживать на лету. С укором к себе согласен, но думаю что смысл дизайна это не портит. В таком случае я бы посоветовал написать собственную реализацию данной функции, поскольку проще реализации придумать сложно, тем более что у нас поиск одноуровневый, чем морочить голову с возможным модифицированием строки, не говоря уже о копировании, что еще хуже. Для таких вещей это слишком. Это еще раз говорит о том, что const_cast тоже весьма порочная практика, если не уверен в том что делаешь. Тоже в раздел по возможности избегать.

Здравствуйте, MaximE, Вы писали:

ME>Я пользовался таким кодом:
ME>

ME>inline uint8_t hex2bin(unsigned char h, unsigned char l)
ME>{
ME>    h |= 0x20;
ME>    h -= 0x30;
ME>    h -= -(h > 9) & 0x27;
ME>    l |= 0x20;
ME>    l -= 0x30;
ME>    l -= -(l > 9) & 0x27;
ME>    return h << 4 | l;
ME>}
ME>

ME>Специально заоптимизирован чтобы не было условных переходов в сгенеренном коде.

Интересно.

Смущают две вещи:

Здесь используются именно unsigned char, а у меня std::string, которая как известно, char. И не обязательно char будет unsigned.

Действительно ли будет разница между твоим кодом и моим при современных оптимизирующих компиляторах?

... << RSDN@Home 1.1.4 stable rev. 510>>

Здравствуйте, Alexmoon, Вы писали:

A>Здравствуйте, Глеб Алексеев, Вы писали:

<skip>

пока писал ответ, то в сразу бросилась в глаза еще одна фактическая ошибка. теоретических пока не вижу.

void CGIParamsBase::Parse(const char *cstr, int iLen)
{
    //-----------------------------------------
    char* pNameBegin = const_cast<char*>(cstr);
    char* pNameEnd   = NULL;

    if (*pNameBegin == '&') pNameBegin++;

    pNameEnd = strtok(pNameBegin, "=&");

    if(pNameEnd == NULL) return;

    //-----------------------------------------
    char* pValueBegin = pNameEnd + 1;
    char* pValueEnd   = NULL;
    
    switch(*pNameEnd)
    {
    case '=':
        pValueEnd = strtok(pValueBegin, "&");
        if (pNameEnd - pNameBegin > 2)
                {
                    //.......................
                    ProcessParam(pNameBegin, pNameEnd - pNameBegin, pConverted, iNewLen);
        }
        
        if(pValueEnd == NULL) break; pNameEnd = pValueEnd + 1;

    case '&':
        Parse(pNameEnd, strlen(pNameEnd));
    }
    
    return;
}

Здравствуйте, индусы, Вы пeсали:

И>Ну, не знаю. Мы так делали:

А вроде в 21 веке живём...

(писал вслепую в браузере, навярняка есть опечатки. но идея, я думаю, понятна

)

        using namеspace boost::spirit;
        X = range_p('A', 'F') | range_p('0', '9');
        Char = ('%' >> (X >> X)[BIND(OnHexChar)] | (range_p((char)31, (char)127) - '=' - '&')[BIND(OnChar)]);
        NVPair =  ch_p('+')[BIN(OnPlusSpace)] | ((+Char)[BIND(OnValue)] >> '=' >>(+Char)[BIND(OnValue)])[BIND(OnNVPair)];
        CGI_Request = NVPair >> *( '&' >> NVPair );

Обладатели SUN Pro C++, VC6.0, G++ 2.95, молчать!
Пишу анонимно, так как на меня сейчас навалятся обладатели этих компилятов

Здравствуйте, <Аноним>, Вы писали:

А>

А>        using namеspace boost::spirit;
А>        X = range_p('A', 'F') | range_p('0', '9');
А>        Char = ('%' >> (X >> X)[BIND(OnHexChar)] | (range_p((char)31, (char)127) - '=' - '&')[BIND(OnChar)]);
А>        NVPair =  ch_p('+')[BIN(OnPlusSpace)] | ((+Char)[BIND(OnValue)] >> '=' >>(+Char)[BIND(OnValue)])[BIND(OnNVPair)];
А>        CGI_Request = NVPair >> *( '&' >> NVPair );
А>

А>Обладатели SUN Pro C++, VC6.0, G++ 2.95, молчать!
А>Пишу анонимно, так как на меня сейчас навалятся обладатели этих компилятов

Да, блин, читабельность просто потрясающая. Не говоря уже о том, что знание boost::spirit совершенно не является обязательным для среднего C++ программиста. Так что сюда нужно еще добавить время, которое мне потребуется на изучение boost::spirit. А заодно и код функций OnChar, OnHexChar, OnPlusSpace, OnValue...

Ты будешь смеятся, но этот код был написан года три назад. И основным требованием к нему было то, что он должен был работать под VC6.0. Заказчик музыку-то заказывает

... << RSDN@Home 1.1.4 stable rev. 510>>

eao197 wrote:

> Да, блин, читабельность просто потрясающая.

Если знать Spirit, то вполне нормально читается. Фактически тот же EBNF

> Ты будешь смеятся, но этот код был написан года три назад. И основным
> требованием к нему было то, что он должен был работать под VC6.0.
> Заказчик музыку-то заказывает

Spirit (частично) работает под VC6. В частности, этот пример там
работать будет.

--
С уважением,
Alex Besogonov (alexy@izh.com)

Posted via RSDN NNTP Server 2.0

On Wed, 30 Nov 2005 16:33:56 -0000, eao197 <31476@users.rsdn.ru> wrote:

> Здравствуйте, MaximE, Вы писали:
>
> ME>Я пользовался таким кодом:
> ME>

> ME>inline uint8_t hex2bin(unsigned char h, unsigned char l)
> ME>{
> ME>    h |= 0x20;
> ME>    h -= 0x30;
> ME>    h -= -(h > 9) & 0x27;
> ME>    l |= 0x20;
> ME>    l -= 0x30;
> ME>    l -= -(l > 9) & 0x27;
> ME>    return h << 4 | l;
> ME>}
> ME>

>
> ME>Специально заоптимизирован чтобы не было условных переходов в сгенеренном коде.
>
> Интересно.

>
> Смущают две вещи:
>
> Здесь используются именно unsigned char, а у меня std::string, которая как известно, char. И не обязательно char будет unsigned.

Это не проблема, т.к. стандартное преобразование signed -> unsigned is well defined, в отличие от обратного. Т.е. реально эта ф-ция пользуется с char строками.

> Действительно ли будет разница между твоим кодом и моим при современных оптимизирующих компиляторах?

На gcc 3/4 с твоим кодом будет branching. Я специально затачивал эту ф-цию чтобы избавиться от branching на gcc в сервере с текстовым протоколом. Обратная ф-ция, также без branching:

inline char* bin2hex(uint8_t b, char* p)
{
     unsigned char h = b >> 4;
     *p++ = '0' + h + (-(h > 9) & ('a' - '9' - 1));
     unsigned char l = b & 0xf;
     *p++ = '0' + l + (-(l > 9) & ('a' - '9' - 1));
     return p;
}

--
Maxim Yegorushkin

Posted via RSDN NNTP Server 2.0

От:	Tom	http://www.RSDN.ru
Дата:	30.11.05 15:48
Оценка:	1 (1)

	От:	srggal
	Дата:	30.11.05 15:49
	Оценка:

От:	eao197	http://eao197.blogspot.com
Дата:	30.11.05 15:51
Оценка:

	От:	srggal
	Дата:	30.11.05 15:51
	Оценка:	+1

	От:	Аноним
	Дата:	30.11.05 15:56
	Оценка:

	От:	Глеб Алексеев
	Дата:	30.11.05 15:59
	Оценка:

От:	Alxndr	http://www.google.com/profiles/alexander.poluektov#buzz
Дата:	30.11.05 16:18
Оценка: