На Visual C++ начал программировать недавно. До этого — только на Борланд С.
Возникла проблема с порчей данных в стеке.
[bДано:[/b]
Метод класса диалогового окна. разбирает строку символов, формирует из каждой пары символов один байт, потом из этих байт — новую цепочку и посылает её в USB-устройство. Для манипуляций с символами в теле метода объявлено три локальных типа char. Работает, но
Трабл:
При выходе их метода возникает ошибка: Runtime check failure #2 — stack around the variable <имя переменной> was corrupted.
Программируя в BC++B с таким не сталкивался, что делать понять не могу, т.к. не ясна причина порчи стэка.
Пытался объявить переменные как char* и выделять/освобождать память напрямую с помощью malloc/free — та же история.
Ошибка исчезает, если объявить эти три переменные как члены класса, но делать этого не хочется.
В чём же ошибка и как её исправить?

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

MD>Трабл:
MD>При выходе их метода возникает ошибка: Runtime check failure #2 — stack around the variable <имя переменной> was corrupted.

Код в студию. Без него гадать можно долго...

Здравствуйте, tarkil, Вы писали:
T>Код в студию. Без него гадать можно долго...

//
// Write data from the host into device 
//
void CLibUsbTestDlg::OnBnClickedWriteDevice()
{
  CDeviceStruct* hDev; 

  int bytesCount = 0;          // bytes counter
  CString newStr = ""; 
  char cTemp, cLowHalfByte;    // for manipulating with characters
    
  CEdit* editField;
  TCHAR               theStr[MAX_STRING_LENGTH]="";
  ULONG               error = ERROR_SUCCESS;
  char*               outPattern;
  char                currentSymbol;  // for construct current byte
  POSM_WDM_LIST_ITEM  ioItem;         // structure for store temporary input-output data

  this->UpdateData(TRUE);       // update control's variables values
  CListBox* listWnd = (CListBox*)GetDlgItem(IDC_DEV_LIST);  // Get DeviceList string pointer
  
  // Get pointer to DeviceInfo structure
  hDev = (CDeviceStruct*)(listWnd->GetItemDataPtr(listWnd->GetCurSel()));  
  this->currentDevice = hDev->usbDevInfo;                   // Get Current USB-device Info structure

  ioItem = (POSM_WDM_LIST_ITEM)malloc(sizeof(OSM_WDM_LIST_ITEM));
  outPattern = (char*)calloc(outputDataSize, 1);            // initialy I used malloc()
  ioItem->OutSize = outputDataSize * outputDataQuantity;    // Number of bytes * multiplicity

  if(ioItem->OutSize > 0)   // if Output data size was defined
  {
    // allocate memory for buffering output data
    ioItem->OutBuffer = (char*)calloc(outputDataSize, outputDataQuantity);  // initialy I used malloc()
    if (ioItem->OutBuffer == NULL)
    {
      MessageBox("Failed to create write buffer", "Error", MB_OK);
      error = ERROR_OUTOFMEMORY;
    }

    // Preparing for write data into bulk pipe
    // Pack pares of symbols in one byte and copy that bytes to buffer
    for (int i = 0; i < outputData.GetLength(); i++)
    {
      strcpy(&currentSymbol, "");
      cTemp = outputData.GetAt(i);              // Get odd symbol
      (void)_stscanf(&cTemp, _T("%x"), &currentSymbol);   // to copying ih hex format

      if (i < outputData.GetLength()-1)                   // if symbol not a last in string
      {
        currentSymbol<<=4;                                // set high half-byte
        cLowHalfByte = outputData.GetAt(++i);             // get even symbol
        (void)_stscanf(&cLowHalfByte, _T("%x"), &cTemp);
        
        currentSymbol^=(cTemp&=0x0F);            // ...and set low half-byte of current byte
        CopyMemory(ioItem->OutBuffer+bytesCount, &currentSymbol, sizeof(currentSymbol));
      }
      else  // symbol is last
        CopyMemory(ioItem->OutBuffer+bytesCount, &currentSymbol, sizeof(currentSymbol));
      bytesCount++;
    }
    bytesCount=0;

    // Write into USB device's pipe-1
    for (int i = 0; i < outputDataQuantity; i++)
      bytesCount += usb_bulk_write(hDev->usbDevHandle, 1, ioItem->OutBuffer, outputDataSize, timeoutValue);

    // Indicate bytes sent count
    editField = (CEdit*)GetDlgItem(IDC_BYTES_SENT_EDIT);
    editField->SetWindowText(newStr);
    newStr.Format("%d", bytesCount);
    AddString(editField, newStr);

    // Free the ioItem memory
    free(outPattern);
    free(ioItem->OutBuffer);
    free(ioItem);
  }
}

Ругань происходит при выходе из метода на переменную currentSymbol. Если объявить её как член класса — то на предыдущие: cTemp, cLowHalfByte.

Hello, MasterDimon!
You wrote on Thu, 23 Dec 2004 10:52:32 GMT:

M> Ругань происходит при выходе из метода на переменную currentSymbol. Если

Ну а почто ты в _stscanf ее адрес передаешь? Ему ж для "%x" адрес от int
надо.

M> объявить её как член класса — то на предыдущие: cTemp, cLowHalfByte.

А вот это и вправду странно — они ж не предыдущие!

With best regards, Sergey.

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

MD>Здравствуйте, tarkil, Вы писали:
T>>Код в студию. Без него гадать можно долго...

MD>      ...
MD>      strcpy(&currentSymbol, "");

Подозрительно выглядящая строчка. Лучше уж currentSymbol = 0; написать. Всё-таки, strcpy для строк придуман, которые char[].

MD>      ...
MD>      (void)_stscanf(&cTemp, _T("%x"), &currentSymbol);   // to copying ih hex format

Вот оно! Формату x в scanf соответствует аргумент типа int, а не char. Соответственно, он в твой 1 байт пишет 4.

MD>      ...
MD>        (void)_stscanf(&cLowHalfByte, _T("%x"), &cTemp);

Аналогично предыдущему.

Всё, больше косяков не углядел.

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

S>Ну а почто ты в _stscanf ее адрес передаешь? Ему ж для "%x" адрес от int надо.
— Почему? у меня там строка символов от 0 до F, каждый символ — это символ, переменная outputData типа CString.
Иначе выполняется двойное преобразование в dec и в устройство пишется всякая туфта.
Попробую, конечно...

Д> объявить её как член класса — то на предыдущие: cTemp, cLowHalfByte.
S>А вот это и вправду странно — они ж не предыдущие!
именно...

Hello, !
You wrote on Thu, 23 Dec 2004 11:08:06 GMT:

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

S>> Ну а почто ты в _stscanf ее адрес передаешь? Ему ж для "%x" адрес от
S>> int надо.
> — Почему?

Потому что scanf так устроен — если у него в форматной стороке написано %x,
то результат он кладет в int. А что там на самом деле, char или double, его
вообще не волнует.

> у меня там строка символов от 0 до F, каждый символ — это символ,
> переменная outputData типа CString. Иначе выполняется двойное
> преобразование в dec и в устройство пишется всякая туфта. Попробую,
> конечно...

Ну за код в целом тут вообще руки сразу отрывать полагается...

With best regards, Sergey.

Здравствуйте, MasterDimon

Ух, как я стормозил! И первый аргумент функции scanf кривой. Ему же не char* нужен, а указатель на zero-terminated string.

Вот это — ацтой:

char currentSymbol;
strcpy(&currentSymbol, "");
(void)_stscanf(&cTemp, _T("%x"), &currentSymbol);

Надо currentSymbol = 0 и не более того. Нельзя вообще объявлять переменную char и затем использовать её адрес в качестве NULL-terminated строки. Буфер под любую осмысленную строку всегда имеет размер минимум 2 байта, потому что в однобайтовый буфер уместится только завершающий ноль — а это будет всегда пустая строка. Строка размером один символ имеет размер два байта

Коррупция наверняка происходит тут (void)_stscanf(&cLowHalfByte, _T("%x"), &cTemp); и во втором случае тоже, потому что %x ожидает целое число (int), а не char. К тому-же, &cLowHalfByte НЕ ЯВЛЯЕТСЯ asciiZ-строкой, потому что функции нужен заверщающий ноль!

И раз уж пишешь на MFC, почему-бы не использовать CString вместо (char*)calloc() и прочих? А CStringA в 7-й версии олично работает в качестве байт-буфера

Я так понял у тебя массив, закодированный шеснадцатеричными половинками: AB800DB6E5CFA4E3EF32134. А чтобы раскодировать один шестнадцатеричный символ, совсем необязательно использовать сложную функцию, предназначенную для чисел произвольной длины. Что-то вроде этого:

inline int decode_hex_dgigit(char hexDigit)
{
return (hexDigit > '9') ? (toupper(hexDigit) - 'A' + 10) : (hexDigit - '0')
}

inline char decode_hex_byte(char first, char last)
{
   return decode_hex_digit(last) + (decode_hex_digit(first) << 4);
}

for (int i = 0, j = 0; i < outputData.GetLength(); i += 2, j++)
   ioItem->OutBuffer[j] = decode_hex_byte( outputData[i], outputData[i + 1] )

достаточно.

S>>Ну а почто ты в _stscanf ее адрес передаешь? Ему ж для "%x" адрес от int надо.
А>- Почему? у меня там строка символов от 0 до F, каждый символ — это символ, переменная outputData типа CString.
А>Иначе выполняется двойное преобразование в dec и в устройство пишется всякая туфта.
А>Попробую, конечно...

scanf предназначен для шестнадцатеричных чисел ПРОИЗВОЛЬНОЙ длины. Чтобы 0A, FFEDBA01, 9A9A9 и прочее парсилось и записывалось в int, long или int64 (смотря какой модификатор присобачишь). А для одного байта в виде XX можно уж не полениться. Причем если там некорректный символ, она ничего не прочитает, и вернет 0 (читай доку по ней). Пробелы она пропускает.

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

S>Ну за код в целом тут вообще руки сразу отрывать полагается...
— А может, лучше всё-таки поучить чайника? Было бы больше пользы

Пойду учиться уму-разуму

Hello, !
You wrote on Thu, 23 Dec 2004 12:00:40 GMT:

S>> Ну за код в целом тут вообще руки сразу отрывать полагается...
> — А может, лучше всё-таки поучить чайника? Было бы больше пользы

Может, и лучше. Только в этом примере почти за каждую строчку кода ругать
надо. Мне лениво. В принципе, со временем сам научится, если не дурак. Я ж
вот вроде научился, хотя когда-то еще страшней вещи писал.

With best regards, Sergey.

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

>> — А может, лучше всё-таки поучить чайника? Было бы больше пользы

S>Может, и лучше. Только в этом примере почти за каждую строчку кода ругать
S>надо. Мне лениво.

Ну-ну, господа, не перегибайте палку. Главная ошибка там в некорректном использовании sscanf. Остальное может и не идеально, но вполне читабельно и работоспособно.

Сказали б такое мне, потребовал бы сатисфакции. На вениках — согласно заветам

Hello, tarkil!
You wrote on Thu, 23 Dec 2004 12:30:44 GMT:

t> Ну-ну, господа, не перегибайте палку. Главная ошибка там в некорректном
t> использовании sscanf. Остальное может и не идеально, но вполне
t> читабельно и работоспособно.

Да-да, особенно читабельны strcpy(&currentSymbol, ""); и
CopyMemory(ioItem->OutBuffer+bytesCount, &currentSymbol,
sizeof(currentSymbol));
Вот это:
[ccode]
editField = (CEdit*)GetDlgItem(IDC_BYTES_SENT_EDIT);
editField->SetWindowText(newStr);
[/code]

вместо SetDlgItemText(""); тоже клево выглядит. Кстати, глядя на следующие
строки возникает мысль — а нафиг он туда вообще пустую строку выводит, может
это и не нужно вовсе? В пользу хорошей читабельности кода также
свидельствует милая сишная привычка объявлять переменные в начале функции
(нафига? оно ж в чистом С все равно не скомпилируется), приводящая к забытым
переменным навроде outPattern. Ну и сишные касты вместо reinterpret_cast и
malloc вместо new (к calloc не прикапываюсь, фиг с ним) на закуску. А, да,
еще б не плохо смартпойнтерами пользоваться — но это уже мелочи. Ну, еще из
3 выделений памяти проверка на 0 только в одном. listWnd->GetCurSel на -1 не
проверяется. Ну и так далее, там в принципе еще недостатков накопать можно.
Сойдет за сатисфакцию?

With best regards, Sergey.

Про malloc и new

в С# new и delete работают отлично. В VC6 как-то столкнулся с тем, что память, выделенная с помощью new, упорно не освобождалась, хотя всё было весьма прозрачно. И пока не вернулся к выделению через malloc и освобождению через free, шла утечка. В VC7 не пытался.

S>В пользу хорошей читабельности кода также свидельствует милая сишная привычка объявлять переменные в начале функции
S>(нафига? оно ж в чистом С все равно не скомпилируется)
— Именно что привычка. Оставшаяся, кстати, ещё с Fortran77

S>Ну, еще из выделений памяти проверка на 0 только в одном.
— Проверки всегда вставляю в последню очередь, сначала добившись, чтобы код работал с устройством. Когда пишешь под железо, не всегда оправдывает себя изначально аккуратное написание с отслеживанием всевозможных исключительных ситуаций

Hello, MasterDimon!
You wrote on Thu, 23 Dec 2004 13:57:45 GMT:

M> Про malloc и new

M> в С# new и delete работают отлично.

А что, в C# уже есть delete?

M> В VC6 как-то столкнулся с тем, что память, выделенная с помощью new,
M> упорно не освобождалась, хотя всё было весьма прозрачно.

Ну, там есть бага с массивами 0 длины. Но, AFAIK, не такая. Еще были баги с
переопределение оператора new для массива объектов. Но в тривиальных случаях
все в шестерке работало нормально.

M> И пока не
M> вернулся к выделению через malloc и освобождению через free, шла утечка.

Что-то слабо верится. Не, в то что бага исчезла верю вполне, но вот в то,
что виноват new — не очень.

M> В VC7 не пытался.
S>> В пользу хорошей читабельности кода также свидельствует милая сишная
S>> привычка объявлять переменные в начале функции (нафига? оно ж в чистом
S>> С все равно не скомпилируется)
M> — Именно что привычка. Оставшаяся, кстати, ещё с Fortran77

Крайне вредная привычка. От которой желательно избавиться.

S>> Ну, еще из выделений памяти проверка на 0 только в одном.
M> — Проверки всегда вставляю в последню очередь, сначала добившись, чтобы
M> код работал с устройством. Когда пишешь под железо, не всегда

Типа бага в драйвере вдвойне приятней? Впрочем, тут гуй.

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

Когда пишешь программы, аккуратность себя оправдывает всегда.

With best regards, Sergey.

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

MD>Про malloc и new

MD>в С# new и delete работают отлично. В VC6 как-то столкнулся с тем, что память, выделенная с помощью new, упорно не освобождалась, хотя всё было весьма прозрачно. И пока не вернулся к выделению через malloc и освобождению через free, шла утечка. В VC7 не пытался.

А зря сдался. Значить проблема так и осталась.

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

S>Hello, tarkil!
S>You wrote on Thu, 23 Dec 2004 12:30:44 GMT:

S>Да-да, особенно читабельны <...>

Практически со всем согласен, за некоторыми замечаниями.

Если сменить malloc на new, то проверки на 0 лишние. Использование/отказ от смартпоинтеров — это скорее вопрос стиля программирования, чем правильности — допустимо и то и то. Использование GetDlgItem/SetWindowText вместо SetDlgItemText это совершенно несущественное замечание, использование переменной вместо константы — нехорошо, но это мелкий криминал Вынос переменных в начало — мне этот стиль не нравится, но есть у него и поклонники, аргументы которых тоже довольно весомы (Вирт — один из самых авторитетных ).

Сишные касты — вопрос спорный и больной. Они гораздо читабельней и я склоняюсь к тому, что их использование вместе со встроенными типами и с указателями на типы, которые стопудово не изменятся (а в указанном примере это условие выполнено) допустимо. В целях как раз увеличения читабельности.

S>Сойдет за сатисфакцию?

Вполне. В таком виде пост стал реально полезным для того, кого обвинили в ламерстве.

А без аргументации — только стреляться! Или колоться. Шпагами, естественно.

Hello, tarkil!
You wrote on Fri, 24 Dec 2004 07:09:13 GMT:

t> Практически со всем согласен, за некоторыми замечаниями.

t> Если сменить malloc на new, то проверки на 0 лишние.
t> Использование/отказ от смартпоинтеров — это скорее вопрос стиля
t> программирования, чем правильности — допустимо и то и то.
Если не забывать про исключения, которые могут кинуть те же MFC, то
использование смартпоинтеров в таком контексте является не вопросом стиля, а
вопросом возможности утечек памяти.

t> Использование GetDlgItem/SetWindowText вместо SetDlgItemText это
t> совершенно несущественное замечание,

Там каст лишний. Если нравится две строчки вместо одной, то следовало
написать хотя бы:

CWnd* editField = GetDlgItem(IDC_BYTES_SENT_EDIT);
if (editField) editField->SetWindowText(newStr);

При этом полезно помнить, что GetDlgItem иногда выделяет память под
временный CWnd, а SetDlgItemText делает это только в случае OLE-контейнеров.

t> использование переменной вместо
t> константы — нехорошо, но это мелкий криминал

Мне тут не понравилось в основном не то, что переменная вместо константы
используется, а то, что она определена черти где и вместо одной понятной
строчки SetDlgItemText("") присутствуют 2, непонятно что в едит выводящие.

t> Вынос переменных в
t> начало — мне этот стиль не нравится, но есть у него и поклонники,
t> аргументы которых тоже довольно весомы (Вирт — один из самых
t> авторитетных ).

Нет у них аргументов Единственный аргумент был когда-то — так
компиляторы писать проще, сразу видно сколько стека под локальные переменные
отъесть.

t> Сишные касты — вопрос спорный и больной. Они гораздо читабельней
t> и я склоняюсь к тому, что их использование вместе со встроенными типами
t> и с указателями на типы, которые стопудово не изменятся (а в указанном
t> примере это условие выполнено) допустимо. В целях как раз увеличения
t> читабельности.

Не бывает типов, которые стопудово не изменятся. А искать при рефакторинге
сишные касты по всей программе — то еще занятие. В то время как плюсовые
касты, по крайней мере, найти не сложно. Насчет читабельности — лично для
меня плюсовые касты читабельней. Они подсвечиваются как ключевые слова и с
ними со скобками разобраться проще.

With best regards, Sergey.