Ошибки новичков или индийский стиль... - Компьютерный форум OSzone.net

Компьютерный форум OSzone.net (http://forum.oszone.net/index.php)

- Программирование и базы данных (http://forum.oszone.net/forumdisplay.php?f=21)

- - Ошибки новичков или индийский стиль... (http://forum.oszone.net/showthread.php?t=184192)

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

Я написал небольшую программку. Хотел бы услышать мнения о том как она смотрится относительно идеологии 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;

}

Ну если прям так придираться, то:
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.

Цитата:

Цитата ganselo

можно было бы использовать только strchr и длину вычислять не с помощью strlen, а вычитанием sPtr от начального адреса. »

Первоначально я так и сделал... для хранения начального адреса была выделена переменная... Но я почему то решил что так будет лучш...
Скажем так... А что если использовать sizeof тоже ищет нуль терминатор как и strlen? Или?!

Код:

sPtr=sizeof(cSect);

Цитата:

Цитата ganselo

касается strcpy, то я лучше бы использовал strncpy »

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

Цитата:

Цитата ganselo

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

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

Цитата:

Цитата ganselo

но забываем вызвать free. »

дамс... промухал

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

Цитата:

Цитата Dr.Dark

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

Лично я завёл для себя правило использовать strncpy вместо strcpy.

P.S

Цитата:

Цитата Dr.Dark

Хотел бы услышать мнения о том как она смотрится относительно идеологии C++ »

Наверно, Вы имели ввиду идеологии Си.

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

Код:

#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;

}

Цитата:

Цитата Dr.Dark

Хотел бы услышать мнения о том как она смотрится относительно идеологии 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

Цитата:

Цитата El Scorpio

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

Полностью согласен, т.к функция pow чтобы возвести в степень использует ряды. Плюс ко всему функция pow считает приближённо (ну это если используете float, double).