Ошибки новичков или индийский стиль...

[Dr.Dark]

Я написал небольшую программку. Хотел бы услышать мнения о том как она смотрится относительно идеологии C++. Т.е. незнаю возможно можно было как то сделать проще или лучше, записать короче и т.д. Зараннее спасиб :) :)

#include <vcl.h>
#include <stdio.h>
#include <iostream.h>
#include <string.h>
#pragma hdrstop

#pragma argsused

int main(int argc, char* argv[])
{
FILE *binFile;
char *cSect, *cKey, *cValue, *sPtr, *kPtr, bSect[512], bKeys[512],
bValue[512], *cFile, MyPath[256], iniPath[256], MyDrive[2], MyDir[251];

cout << "Patcher started...\r\n";
GetModuleFileName(NULL,MyPath,sizeof(MyPath));
_splitpath(MyPath, MyDrive, MyDir, NULL, NULL);
sprintf(iniPath,"%s%s%s", MyDrive,MyDir,"map.ini");
sprintf(MyPath,"%s%s", MyDrive,MyDir);
cout << "Getting patch map...\r\n";
if (GetPrivateProfileString(NULL,NULL,FALSE,bSect,sizeof(bSect),iniPath) == 0)
{
cout << "Can't see map.ini\r\n" << "\r\n";
system("pause");
exit(1);
}

sPtr=bSect;
while(*sPtr!=NULL)//перебираем буфер секций
{
cSect=(char*) malloc(strlen(sPtr)+1);
strcpy(cSect,sPtr);
sPtr=strchr(sPtr,'\0')+1;
cFile=(char*) malloc(strlen(MyPath)+strlen(cSect)+1);
sprintf(cFile,"%s%s",MyPath,cSect);
binFile=fopen(cFile,"r+b");
printf("Patching %s:\r\n",cSect);
if (binFile==NULL)
{
printf("\tCan't open file %s\r\n",cSect);
system("pause");
exit(2);
}
GetPrivateProfileString(cSect,NULL,FALSE,bKeys,sizeof(bKeys),iniPath);
kPtr=bKeys;
while(*kPtr!=NULL) //перебираем ключи секции
{
cKey=(char*) malloc(strlen(kPtr)+1);
strcpy(cKey, kPtr);
kPtr=strchr(kPtr,'\0')+1;
GetPrivateProfileString(cSect,cKey,FALSE,bValue,sizeof(bValue),iniPath);
cValue=(char*) malloc(strlen(bValue)+1);
strcpy(cValue,bValue);
printf("\t%s=%s\r\n",cKey,cValue);
fseek(binFile,atoi(cKey),0);
fwrite(cValue, 1, strlen(cValue), binFile);
};
fclose(binFile);
}
cout << "Patching done. Have a nice day :)...\r\n";
return 0;
}

[ganselo]

Ну если прям так придираться, то:
1.

cSect=(char*) malloc(strlen(sPtr)+1);
strcpy(cSect,sPtr);
sPtr=strchr(sPtr,'\0')+1;

я бы этот кусок как нибудь упростил, т.к тут используются 2 функции (strlen, strchr) которые совершают 2*N итераций, хотя можно было бы использовать только strchr и длину вычислять не с помощью strlen, а вычитанием sPtr от начального адреса.
Что, касается strcpy, то я лучше бы использовал strncpy, т.к. переполнение буфера не кто не отменял (хотя тут мб и не будет такого...).

2.

fseek(binFile,atoi(cKey),0);

Уверены, что в cKey всегда будет строка из цифр?

3.
Вызываем malloc, но забываем вызвать free.

[Dr.Dark]

можно было бы использовать только strchr и длину вычислять не с помощью strlen, а вычитанием sPtr от начального адреса. »
Первоначально я так и сделал... для хранения начального адреса была выделена переменная... Но я почему то решил что так будет лучш...
Скажем так... А что если использовать sizeof тоже ищет нуль терминатор как и strlen? Или?!
sPtr=sizeof(cSect);

касается strcpy, то я лучше бы использовал strncpy »
ИМХО тут его не будет потому что память выделяю под длину строки...

Уверены, что в cKey всегда будет строка из цифр? »
Нет эт так пробный вариант... Там будет строка HEX смещение от начала файла.... Ща набросаю функцию перевод char* в hex и там проверять буду...

но забываем вызвать free. »
дамс... промухал

[ganselo]

С помощью операции sizeof можно определить размер памяти которая соответствует идентификатору или типу. Если вы выделяете память char *c = malloc(...), то sizeof(c) вернёт, 1 байт. Так, что sizeof тут не прокатит.

ИМХО тут его не будет потому что память выделяю под длину строки... »
Лично я завёл для себя правило использовать strncpy вместо strcpy.

P.S
Хотел бы услышать мнения о том как она смотрится относительно идеологии C++ »
Наверно, Вы имели ввиду идеологии Си.

[Dr.Dark]

Я ещё немног подумал над алгоритмом и немного упростил всё...
терь так выглядит

#include <vcl.h>
#include <stdio.h>
#include <iostream.h>
#include <string.h>
#include <math.h>
#pragma hdrstop

#pragma argsused
int hstoi(char* hexstr)
{
int idx, len, result;
len=strlen(hexstr);
strrev(hexstr);
result=0;

for(idx=0; idx<len; idx++)
{
if (isdigit(hexstr[idx]))
{
result+=(hexstr[idx]-48)*pow(16,idx);
} else {
switch(toupper(hexstr[idx]))
{
case 'A': result+=10*pow(16,idx);break;
case 'B': result+=11*pow(16,idx);break;
case 'C': result+=12*pow(16,idx);break;
case 'D': result+=13*pow(16,idx);break;
case 'E': result+=14*pow(16,idx);break;
case 'F': result+=15*pow(16,idx);break;
default:
cout << "Incorrect offset";
exit(3);
break;
}
}
}
return result;
}

int main(int argc, char* argv[])
{
FILE *binFile;
char *sPtr, *kPtr, *cFile, bSect[512], bKeys[512], bValue[512], MyPath[256],
iniPath[256], MyDrive[2], MyDir[251];

cout << "Patcher started...\r\n";
GetModuleFileName(NULL,MyPath,sizeof(MyPath));
_splitpath(MyPath, MyDrive, MyDir, NULL, NULL);
sprintf(iniPath,"%s%s%s", MyDrive,MyDir,"map.ini");
sprintf(MyPath,"%s%s", MyDrive,MyDir);
cout << "Getting patch map...\r\n";
if (GetPrivateProfileString(NULL,NULL,FALSE,bSect,sizeof(bSect),iniPath) == 0)
{
cout << "Can't see map.ini\r\n" << "\r\n";
system("pause");
exit(1);
}

sPtr=bSect;
while(*sPtr!=NULL)//перебираем буфер секций
{
cFile=(char*) malloc(strlen(MyPath)+strlen(sPtr)+1);
sprintf(cFile,"%s%s",MyPath,sPtr);
binFile=fopen(cFile,"r+b");
printf("Patching %s:\r\n",sPtr);
if (binFile==NULL)
{
printf("\tCan't open file %s\r\n",sPtr);
system("pause");
exit(2);
}
GetPrivateProfileString(sPtr,NULL,FALSE,bKeys,sizeof(bKeys),iniPath);
kPtr=bKeys;
while(*kPtr!=NULL) //перебираем ключи секции
{
GetPrivateProfileString(sPtr,kPtr,FALSE,bValue,sizeof(bValue),iniPath);
printf("\t%s=%s\r\n",kPtr,bValue);
fseek(binFile,hstoi(kPtr),0);
fwrite(bValue, 1, strlen(bValue), binFile);
kPtr=strchr(kPtr,'\0')+1;
};
fclose(binFile);
free(cFile);
sPtr=strchr(sPtr,'\0')+1;
}
cout << "Patching done. Have a nice day :)\r\n";
return 0;
}

[El Scorpio]

Хотел бы услышать мнения о том как она смотрится относительно идеологии C++. »
Идеология С++ предполагает использование классов строковых контейнеров
Ну и конечно же,*если всё-таки хочется выделять память "руками", но нельзя забывать её освобождать

замечания к функции int hstoi(char* hexstr)
1. Поскольку hexstr не изменяется, нужно использовать указатель на константу (иначе функция не сможет принимать константы в качестве аргументов)
2. В*цикле много раз выполняется операция pow(16,idx). Это жуткий "индуизм" :), проще каждый раз в цикле выполнять операцию умножения.
for(int idx=0, powdx=1; idx<len; idx++, powdx *= 16)
А поскольку у нас значение кратное степеням двойки, то ещё быстрее будет логический сдвиг единицы в числе.
3. Мегабаг. Если в функцию будет направлена длинная строка, то произойдёт целочисленное переполнение значения powdx (равно как и результата исходного вычисления). Нужно добавить проверку значения len

[ganselo]

2. В*цикле много раз выполняется операция pow(16,idx). Это жуткий "индуизм" , проще каждый раз в цикле выполнять операцию умножения. »
Полностью согласен, т.к функция pow чтобы возвести в степень использует ряды. Плюс ко всему функция pow считает приближённо (ну это если используете float, double).