Re: Стиль написания
От: Shmakov Россия  
Дата: 21.09.06 13:11
Оценка: -4
Венгерка хде?
Re: Стиль написания
От: korzhik Россия  
Дата: 21.09.06 13:28
Оценка: 54 (6) +3
Здравствуйте, Аноним, Вы писали:

А>Здравствуйте мастера =)


А>Я начинающий кодер на cpp, и хочу оценить свой стиль написания.. возможно мне нужно его поменять. Думаю ни для кого не секрет что удача в кодинге напрямую зависит от стиля.


А>Собственно, вот здесь находится кусок моего кода, хотелось бы услышать, что вы об этом думаете :D


А>1stapp.cpp


не смог пройти равнодушно мимо такого безобразия!
Итак:
1. Чего мы там пишем в секцию кода? Хакер, ты наш, юный.
2. Отступы должны быть аккуратными
3. Не мешало бы разбить главную функцию, на несколько.
В твоём случае могло бы быть так:
  void create_and_mapping_file(const char*);
  bool check_PE_header();
  void make_some_shit_with_codesection();


4. у переменных должны быть более осмысленные имена

5. если уж используешь коды возврата, то будь добр проверяй все возвращаемые значение и освобождай все ресурсы

твой код:
HANDLE fhandle = CreateFile(Victim,GENERIC_READ+GENERIC_WRITE,FILE_SHARE_READ,NULL,OPEN_EXISTING,FILE_ATTRIBUTE_NORMAL,NULL);
if(fhandle==INVALID_HANDLE_VALUE) {
    MessageBox(NULL,mbFOpen,mbErrTitle,MB_ICONERROR);
    return 0;
};

DWORD fsize = GetFileSize(fhandle,NULL);
HANDLE maphandle = CreateFileMapping(fhandle,NULL,PAGE_READONLY,NULL,NULL,NULL);
DWORD mfile = (DWORD)MapViewOfFile(maphandle,FILE_MAP_READ,NULL,NULL,NULL);
DWORD vmem = (DWORD)VirtualAlloc(NULL,fsize+4096,MEM_COMMIT,PAGE_READWRITE);


как надо бы:
    HANDLE file = CreateFile(file_name, 
                             GENERIC_READ | GENERIC_WRITE, 
                             FILE_SHARE_READ, 
                             NULL,
                             OPEN_EXISTING,
                             FILE_ATTRIBUTE_NORMAL,
                             NULL);

    if (file == INVALID_HANDLE_VALUE) 
    {
        MessageBox(NULL, mbFOpen, mb_title, MB_ICONERROR);
        return 0;
    }

    DWORD  file_size = GetFileSize(file, NULL);
    if (file_size == INVALID_FILE_SIZE)
    {
        return 0;
    }

    HANDLE file_mapping = CreateFileMapping(file, NULL, PAGE_READONLY, NULL, NULL, NULL);
    if (file_mapping == 0)
    {
        CloseHandle(file);
        return 0;
    }
    void*  file_base = MapViewOfFile(file_mapping, FILE_MAP_READ, NULL, NULL, NULL);

    if ( file_base == 0 )
    {
        CloseHandle(file_mapping);
        CloseHandle(file);
        return 0;
    }
    void* vmem = VirtualAlloc(NULL, file_size + 4096, MEM_COMMIT, PAGE_READWRITE);

    if (vmem == 0)
    {
        UnmapViewOfFile(file_base);
        CloseHandle(file_mapping);
        CloseHandle(file);
        return 0;
    }


но это тоже плохо, так что читай про RAII

6. Чегой то за цикл?
while (d2!=0) {
    *(BYTE*)d0 = *(BYTE*)mfile;
    d0 = d0+1;
    mfile = mfile+1;
    d2 = d2-1;};


используй memcpy

7. дальше в коде идут непонятные циферки и буковки.
А вот чтобы они были понятными, надо вместо циферок использовать правильно именнованные константы. И ещё программист должен уметь грамотно подбирать инструмент.
Здесь ты анализируешь PE заголовок, так используй файл winnt.h (уже включён через windows.h)
там определены все структуры и константы для работы с PE
Например твой код:
WORD w0 = *(WORD*)vmem;
if(*(WORD*)(vmem)==0x5A4D) {
    DWORD PEstart = *(DWORD*)(vmem+60);
    if(*(DWORD*)(vmem+PEstart)==0x4550) {
        PEstart += (DWORD)(vmem);
        DWORD EP = *(DWORD*)(PEstart+40);
        DWORD sVA = *(DWORD*)(PEstart+0x104);
        if(sVA>EP) {MessageBox(NULL,mbEPAErr,mbErrTitle,MB_ICONERROR);}
        else {
            DWORD sVS = *(DWORD*)(PEstart+0x100);
            d0 = sVS+sVA;
            if(d0<=EP) {MessageBox(NULL,mbEPAErr,mbErrTitle,MB_ICONERROR);}
            else {


должен бытьпримерно таким:
    PIMAGE_DOS_HEADER dosHeader = (PIMAGE_DOS_HEADER)file_base;
    if (dosHeader->e_magic == IMAGE_DOS_SIGNATURE)
    {
        PIMAGE_NT_HEADERS pNTHeader; 
        pNTHeader = (PIMAGE_NT_HEADERS)(dosHeader + dosHeader->e_lfanew);

        if (pNTHeader->Signature == IMAGE_NT_SIGNATURE)
        {
            PIMAGE_SECTION_HEADER  pSectionHeader;
            pSectionHeader = (PIMAGE_SECTION_HEADER)(pNTHeader + sizeof(IMAGE_NT_HEADERS));
            DWORD EP = pNTHeader->OptionalHeader.BaseOfCode;
            DWORD sVA = pSectionHeader->Misc.VirtualSize;

            if (sVA > EP) 
            {
                MessageBox(NULL, mbEPAErr, mb_title, MB_ICONERROR);
            }
            else 
            {

и выделен в отдельную функцию, что то типа check_PE()
Re[2]: Стиль написания
От: demi США  
Дата: 21.09.06 14:48
Оценка:
Здравствуйте, korzhik, Вы писали:
K>как надо бы:
K>
K>    HANDLE file = CreateFile(file_name, 
K>    if (file == INVALID_HANDLE_VALUE) 
K>        return 0;
K>    HANDLE file_mapping = CreateFileMapping(file, NULL, PAGE_READONLY, NULL, NULL, NULL);
K>    if (file_mapping == 0)
K>    {
K>        CloseHandle(file);
K>        return 0;
K>    }
K>    void*  file_base = MapViewOfFile(file_mapping, FILE_MAP_READ, NULL, NULL, NULL);
K>    if ( file_base == 0 )
K>    {
K>        CloseHandle(file_mapping);
K>        CloseHandle(file);
K>        return 0;
K>    }
K>    void* vmem = VirtualAlloc(NULL, file_size + 4096, MEM_COMMIT, PAGE_READWRITE);
K>    if (vmem == 0)
K>    {
K>        UnmapViewOfFile(file_base);
K>        CloseHandle(file_mapping);
K>        CloseHandle(file);
K>        return 0;
K>    }
K>


Здесь может быть не все так жестко, а так:

HANDLE hfile = CreateFile(...);
if (INVALID_HANDLE_VALUE == hfile) //Кстати, уважаемый начинающий, догадаетесь, почему я hfile написал справа от знака сравнения.
  return 0;

HANDLE hMap = CreateFileMapping(...);
CloseHandle(hfile); //BEEF! <-- Здесь можно так сделать!!! Дело в том что Винда не отпустит хендл, пока он используется.

if (!hMap)
  return 0;

PVOID pMap = MapViewOfFile(...);
CloseHandle(hMap); //BEEF! <-- Здесь можно так сделать!!! Дело в том что Винда не отпустит хендл, пока он используется.

if (!pMap)
  return 0;

//... наши великие дела ...

//Внутри все работает на счетчиках ссылок. Отпустили mapping и хендлы освободились автоматически Виндой.
UnmapViewOfFile(pMap);


А вот у меня дор сих пор детская проблема — люблю выравнивание в столбик... Например:

//Бесит что держите семеро :)
#define WIDTH 800
#define HEIGHT 600
//А вот так нравится...
#define DIMX 800
#define DIMY 600
//И к какому врачу идти не знаю....
Не стыдно попасть в дерьмо, стыдно в нём остаться!
Re[3]: Стиль написания
От: korzhik Россия  
Дата: 21.09.06 15:41
Оценка: +2 :)
Здравствуйте, demi, Вы писали:


D>Здесь может быть не все так жестко, а так:


D>
D>HANDLE hfile = CreateFile(...);
D>if (INVALID_HANDLE_VALUE == hfile) //Кстати, уважаемый начинающий, догадаетесь, почему я hfile написал справа от знака сравнения.
D>  return 0;

вот кстати спорная привычка

D>HANDLE hMap = CreateFileMapping(...);
D>CloseHandle(hfile); //BEEF! <-- Здесь можно так сделать!!! Дело в том что Винда не отпустит хендл, пока он используется.

первый раз такое вижу, думаю возможны какие нибудь подводные камни

D>//Внутри все работает на счетчиках ссылок. Отпустили mapping и хендлы освободились автоматически Виндой.
D>UnmapViewOfFile(pMap);

возможно. я не в курсе

D>


D>А вот у меня дор сих пор детская проблема — люблю выравнивание в столбик... Например:


D>
D>//Бесит что держите семеро :)
D>#define WIDTH 800
D>#define HEIGHT 600
D>//А вот так нравится...
D>#define DIMX 800
D>#define DIMY 600
D>//И к какому врачу идти не знаю....
D>


такаяж фигня, люблю тоже всё выравнивать
Re[3]: Стиль написания
От: Roman Odaisky Украина  
Дата: 21.09.06 16:51
Оценка:
Здравствуйте, demi, Вы писали:

//Кстати, уважаемый начинающий, догадаетесь, почему я hfile написал справа от знака сравнения.

Я тоже когда-то так писал, потом перестал. Компилятор всё равно предупредит (3 >= /W? Тогда мы идем к вам!), а читаемость всё же падает. Читаем-то мы слева направо.
if(FAIL == {здесь какое-то длинное выражение}) // если что? исключительная ситуация?
if(FAIL == promptUser("Format drive C:?", RETRY | IGNORE | FAIL)) // а-а, это кнопка!

if(promptUser({здесь строки всякие, константы}... // ага, задаем какой-то вопрос, и в зависимости от реакции делаем то или иное
if(promptUser("Format drive C:?", RETRY | IGNORE | FAIL) == FAIL) // понятно

Да и в случае сравнений «<»/«>»/... нарушается или концепция, или здравый смысл.
if(SOME_VALUE == f()) // это еще можно понять...

if(SOME_THRESHOLD <= readItTenTimesToUnderstand()) // ??!
if(thisIsJustABitSimpler() >= SOME_THRESHOLD) // а это расходится со стратегией для ==.
До последнего не верил в пирамиду Лебедева.
Re[4]: Стиль написания
От: demi США  
Дата: 22.09.06 09:00
Оценка:
Здравствуйте, Roman Odaisky, Вы писали:

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


RO>
//Кстати, уважаемый начинающий, догадаетесь, почему я hfile написал справа от знака сравнения.

RO>Я тоже когда-то так писал, потом перестал. Компилятор всё равно предупредит (3 >= /W? Тогда мы идем к вам!), а читаемость всё же падает. Читаем-то мы слева направо.

Согласен. Другое дело, что хотелось бы, что бы многоуважаемый начинающий знал об этом. Я так тож редко делаю и пишу hr == E_FAIL а не наоборот, (а вообще пишу FAILED(hr)). Это просто, к слову.
Не стыдно попасть в дерьмо, стыдно в нём остаться!
Re[4]: Стиль написания
От: demi США  
Дата: 22.09.06 09:03
Оценка:
Здравствуйте, korzhik, Вы писали:

D>>
D>>HANDLE hMap = CreateFileMapping(...);
D>>CloseHandle(hfile); //BEEF! <-- Здесь можно так сделать!!! Дело в том что Винда не отпустит хендл, пока он используется.

K>первый раз такое вижу, думаю возможны какие нибудь подводные камни

D>>//Внутри все работает на счетчиках ссылок. Отпустили mapping и хендлы освободились автоматически Виндой.
D>>UnmapViewOfFile(pMap);

K>возможно. я не в курсе

D>>


Этот код я впервые с пояснениями увидел не где-нибудь, а в boost. Так что посмотрите, там сказано с оговоркой 'seems to be'. Но если такая уважаемая либа делает так, думаю, это не так плохо (наверно кто-то из создателей знает что в потрохах Windows).
Не стыдно попасть в дерьмо, стыдно в нём остаться!
Re: Стиль написания
От: Ubivetz Украина  
Дата: 22.09.06 09:40
Оценка:
Здравствуйте, Аноним, Вы писали:

А>Здравствуйте мастера =)


А>Я начинающий кодер на cpp, и хочу оценить свой стиль написания.. возможно мне нужно его поменять. Думаю ни для кого не секрет что удача в кодинге напрямую зависит от стиля.


А>Собственно, вот здесь находится кусок моего кода, хотелось бы услышать, что вы об этом думаете :D


А>1stapp.cpp

Лично я бы порекомендовал:
1) использовать константы с осмысленными именами, а не числовые значения,
2) комментировать назначение блоков кода,
3) отделять блоки кода, несущие определённую смысловую нагрузку, пустыми строками,
4) всё же располагать операторные скобки друг под другом (несколько облегчает чтение),
5) (на будущее) не сильно увлекаться шаблонами, да и вообще, поменьше извращаться (иначе коментировать извраты)
6) писать код в расчёте скорее на новичка или программиста средней категории, нежели на гуру.
Эх, люблю выпить и переспать с кем нибудь!
Но чаще выходит перепить с кем — нибудь и выспаться...
Re[3]: Стиль написания
От: asdfghjkl  
Дата: 22.09.06 10:58
Оценка:
D>
D>HANDLE hfile = CreateFile(...);
D>if (INVALID_HANDLE_VALUE == hfile) //Кстати, уважаемый начинающий, догадаетесь, почему я hfile написал справа от знака сравнения.
D>


Ну ладно мы мучаеся, читая наизнанку, но молодежи-то зачем это грузить?
Невозможно чтобы у всех было всё, так как всех много, а всего мало...
Re[4]: Стиль написания
От: demi США  
Дата: 22.09.06 14:17
Оценка:
Здравствуйте, asdfghjkl, Вы писали:

D>>
D>>HANDLE hfile = CreateFile(...);
D>>if (INVALID_HANDLE_VALUE == hfile) //Кстати, уважаемый начинающий, догадаетесь, почему я hfile написал справа от знака сравнения.
D>>


A>Ну ладно мы мучаеся, читая наизнанку, но молодежи-то зачем это грузить?


Roman Odaisky заметил неудобочитаемость данной конструкции. Я не отрицаю.

Кстати, а никто не имеет ничего против названия типа _name данных в классе? (в POD и public не практикую). Вроде по стандарту C++ имена с двумя подчеркиваниями подряд в любом месте слова забронированы, _+Заглваная буква, _+любой символ ТОЛЬКО для глобальных имен. Получается _name в классе безвреден. Лично мне m_ не нравится сильно. Как и венгерка!!! Эта вообще ахтунг! Венгерку ф топку!!!

class A
{
   int _num;
   int _totalSize;
   //so on...
};
Не стыдно попасть в дерьмо, стыдно в нём остаться!
Re[5]: Стиль написания
От: kan Великобритания  
Дата: 22.09.06 15:05
Оценка:
demi wrote:

> Кстати, а никто не имеет ничего против названия типа _name данных в

> классе? (в POD и public не практикую). Вроде по стандарту C++ имена с
> двумя подчеркиваниями подряд в любом месте слова забронированы,
> _+Заглваная буква, _+любой символ ТОЛЬКО для глобальных имен. Получается
> _name в классе безвреден. Лично мне m_ не нравится сильно. Как и
> венгерка!!! Эта вообще ахтунг! Венгерку ф топку!!!
Да можно и "m_", в бусте иногда попадается "fieldName_", а вариант "_fieldName" я видел только в языках не
обеспечивающих security для обозначения приватных данных или для нестандартных расширений.
Posted via RSDN NNTP Server 2.0
но это не зря, хотя, может быть, невзначай
гÅрмония мира не знает границ — сейчас мы будем пить чай
Re: Стиль написания
От: sugarde  
Дата: 22.09.06 15:16
Оценка:
Здравствуйте, Аноним, Вы писали:

Прсотите, если не секрет, какой декомпилятор выплюнул этот код?
В жизни кaждoгo челoвекa бывaют приятные мoменты, кoгдa oн чувствует себя пoлным идиoтoм. Приятнoсть этих мoментoв в пoстижении истины.
Re[5]: Стиль написания
От: Roman Odaisky Украина  
Дата: 22.09.06 21:40
Оценка:
Здравствуйте, demi, Вы писали:

D>Кстати, а никто не имеет ничего против названия типа _name данных в классе? (в POD и public не практикую). Вроде по стандарту C++ имена с двумя подчеркиваниями подряд в любом месте слова забронированы, _+Заглваная буква, _+любой символ ТОЛЬКО для глобальных имен. Получается _name в классе безвреден. Лично мне m_ не нравится сильно. Как и венгерка!!! Эта вообще ахтунг! Венгерку ф топку!!!


D>
D>class A
D>{
D>   int _num;
D>   int _totalSize;
D>   //so on...
D>};
D>

Можно, разумеется. А по поводу HN зря. Читал статью Джоэла?

По поводу именования — я обычно делаю так:
class X
{
. . .
private:
    int i_;
    std::deque<std::string> strings_;
    Y allTheMembersAreCalledThisWay_;

public:
    void regularMemberFunction();
    void _somethingQuestionable(); // «handle with care!»
};

Т. о., члены именуются с подчеркиванием в конце, методы — без оного, дебажно-грабельные классы начинают свои имена с подчеркивания, что призвано привлекать внимание. Вот например:
template <class ClassBeingTested>
class InvariantCheckHelper: public InvariantCheckHelperBase
{
public:
    InvariantCheckHelper(ClassBeingTested const& obj): obj_(obj)
    {
    }

    ~InvariantCheckHelper()
    {
        obj_._checkInvariants();
    }

private:
    ClassBeingTested const& obj_;
};

template <class ClassBeingTested>
InvariantCheckHelper<ClassBeingTested> makeInvariantCheckHelper(ClassBeingTested const& obj)
{
    return InvariantCheckHelper<ClassBeingTested>(obj);
}

#define CHECK_INVARIANTS_NOW_AND_ON_EXIT() \
    ; \
    this->_checkInvariants(); \
    InvariantCheckHelperBase const& invariantCheckHelper = makeInvariantCheckHelper(*this); \
    (void)invariantCheckHelper;

использование, полагаю, очевидно:
class X
{
    void _checkInvariants() const
    {
        assert(abc);
        assert(xyz);
    }

    void whatever()
    {
        CHECK_INVARIANTS_NOW_AND_ON_EXIT();
        doSomething();
    }
};
До последнего не верил в пирамиду Лебедева.
Re[6]: Стиль написания
От: demi США  
Дата: 23.09.06 07:33
Оценка:
Здравствуйте, Roman Odaisky, Вы писали:
RO>А по поводу HN зря. Читал статью Джоэла?
Почитаю внимательно, когда освобожусь.

Вот мои мысли против HN.

Мне не нравиться удлинение имен. Во вторых, усложенение рефакторинга. DWORD dwSize; -> WORD wSize; И все вхождения 'dwSize' надо менять. Зачастую этим пренебрегают, и имена не соответствуют типу, что еще хуже. Так можно обозначить ограниченный набор типов (не user-defined). Это было хорошим решением для asm-кода и местах сопряжения asm-C, где никаких проверок типов нет и отсутствуют "разумные" преобразования.

;Давно не практиковался а асме, не пинайте сильно. Но суть ясна.
dw  word_size_var; ;2 bytes
dd  dword_var;     ;4 bytes

;...

;Забыли что переменная 2 байта, но компилятор съест - возьмет лишних 2 байта.
mov eax, DWORD PTR word_size_var;


Факт ведь, что MS отказалась от HN. К тому же, мы живем не в мире Borland Turbo C 3.1 а в мире VisualStudio + VisualAssist, и для того, чтобы узнать тип переменной, достаточно просто навести на имя мышкой. А если стоит ассист, то вообще ничего делать не надо — он когда редактируешь выводит в своей строке определение символа под курсором редактора(для переменной тип). + ворнинги и все такое Венгерка излишня и утомительна.

А вот различить глобальная, стековая или член класса данная перменная, сложнее и код читается лучше, если видно, где функция обращается к членам класса, stack frame и тд. Так что я за _name (beef!), m_name, name_ в классах, g_name, ::name для глобальных переменных. Остальные — как угодно, но БЕЗ венгерки.
Не стыдно попасть в дерьмо, стыдно в нём остаться!
Re[6]: Стиль написания
От: demi США  
Дата: 23.09.06 07:37
Оценка:
Здравствуйте, Roman Odaisky, Вы писали:
А вообще мой стиль отсюда — здесь. C# naming conventions, но для C++ по мойу очень удачно подходит.
Не стыдно попасть в дерьмо, стыдно в нём остаться!
Re: Стиль написания
От: strcpy Россия  
Дата: 23.09.06 08:25
Оценка:
А>Я начинающий кодер на cpp, и хочу оценить свой стиль написания.. возможно мне нужно его поменять. Думаю ни для кого не секрет что удача в кодинге напрямую зависит от стиля.
Для начала определись с правилами кодирования, в частоности, выбери, что ты используешь — пробелы или табуляции.
Удвой число ошибок, если не получается добиться цели.
Re: Стиль написания
От: Анатолий Широков СССР  
Дата: 23.09.06 09:27
Оценка:
А>1stapp.cpp

Качественный фейк.
Re[6]: Стиль написания
От: trophim Россия  
Дата: 24.09.06 20:45
Оценка: +1
Здравствуйте, Vladimir D Belousov, Вы писали:

VDB>Самые распространенные забыли:

VDB>// Durty HACK
VDB>// TODO
VDB>(забавно, у меня такие комментарии Kate особо подствечивает).

VDB>Без таких комментариев можно всё сломать, решив что это (то или иное) ошибка и её срочно надо исправить.


Да, есть такое дело. А комменты TODO, я думаю следует писать сразу же и писать то, чего собственно туду, а то уже через час забываешь в каком месте ты чего-то не дописал. В итоге очень интересно наблюдать как вызывается метод базового класса, который вроде уже реализован, нет никаких в нем TODO, а он, собака, как-то ведет себя подозрительно. Забыл записать TODO, вот и не было куска логики. Смотрел код. Долго думал.
[EOF]
Let it be! — Давайте есть пчелу!
Re[2]: Стиль написания
От: trophim Россия  
Дата: 24.09.06 20:45
Оценка: :)
Здравствуйте, <Аноним>, Вы писали:

А>гавнакод.


Хм, хороший термин. Возьму на вооружение. Не возражаете?
А то Фаулеровский "bad smells in code" не так точен.
[EOF]
Let it be! — Давайте есть пчелу!
Re[4]: Стиль написания
От: trophim Россия  
Дата: 24.09.06 20:45
Оценка: 1 (1)
Здравствуйте, korzhik, Вы писали:

D>>
D>>//Бесит что держите семеро :)
D>>#define WIDTH 800
D>>#define HEIGHT 600
D>>//А вот так нравится...
D>>#define DIMX 800
D>>#define DIMY 600
D>>//И к какому врачу идти не знаю....
D>>


K>такаяж фигня, люблю тоже всё выравнивать


Нас уже трое.
[EOF]
Let it be! — Давайте есть пчелу!
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.