Bu makro suiiste'molmi?

Chunki u bir xil emas. Agar siz xuddi shu narsani funktsiyaga qo'ysangiz, X -1 ga teng bo'lsa, DCD_BADWRITE ni qaytaradigan qisqa funksiyaga ega bo'lasiz. Biroq, ko'rib chiqilayotgan makros o'z ichiga olgan funksiyadan qaytadi. Misol:

int foobar(int value) {
    CHECK_FREAD(value, "Whatever");
    return 0;
}

kengaytiriladi:

int foobar(int value) {
    if (value == -1) {
        return (DCD_BADWRITE);
    };
    return 0;
}

Agar bu funktsiya bo'lsa, tashqi funktsiya (foobar) natijani mamnuniyat bilan e'tiborsiz qoldiradi va 0 ni qaytaradi.

Funktsiyaga juda o'xshab ketadigan, lekin hal qiluvchi tarzda o'zini boshqacha tutadigan so'l asosiy IMO yo'q. Men ha, deyman, bu makrosni suiiste'mol qilish.

Buni funksiya sifatida bajarish mumkin emas, chunki return makro ichidagi blok emas, balki chaqiruv funksiyasidan qaytmoqda.

Bu makrolarga yomon nom beradigan narsadir, chunki u boshqaruv mantiqining muhim qismini yashiradi.

Men bunday so'lni *abuse* deb atash kerakmi, deb o'ylayapman, lekin bu yaxshi fikr emasligini aytaman.

Umuman olganda, siz makroda return dan foydalanishdan qochishingiz kerak deb bahslashaman. Qaytishni to'g'ri bajaradigan so'l yaratish qiyin, uni noqulay yoki tushunish qiyin deb ataydigan funktsiyani boshqarish oqimini yaratmasdan. Siz o'zingiz bilmagan holda funktsiyadan "tasodifan" qaytishni xohlamaysiz. Bu disk raskadrovka uchun zerikarli bo'lishi mumkin.

Bundan tashqari, ushbu makro qanday ishlatilishiga qarab kod oqimi bilan bog'liq muammolarga olib kelishi mumkin. Masalan, kod

if (some_condition)
    CHECK_FREAD(val, "string")
else
    something_else();

siz o'ylagan narsani qilmaydi (else makro ichidagi if bilan bog'lanadi). Atrofdagi shartlarni o'zgartirmasligiga ishonch hosil qilish uchun so'l { } ichiga kiritilishi kerak.

Bundan tashqari, ikkinchi argument ishlatilmaydi. Bu erda aniq muammo - bu hujjatlarga mos kelmaydigan dastur. Yashirin muammo, so'l quyidagi tarzda chaqirilganda:

char* messages[4];          // Array of output messages
char* pmsg = &messages[0];
....
CHECK_FREAD(val, pmsg++);

Makros chaqirilgandan so'ng ko'rsatgich massivning keyingi elementiga o'tishini kutasiz. Biroq, ikkinchi parametr ishlatilmaydi, shuning uchun o'sish hech qachon sodir bo'lmaydi. Bu yon ta'siri bo'lgan bayonotlar makrolarda muammolarga olib kelishining yana bir sababidir.

Bularning barchasini bajarish uchun so'ldan foydalanish o'rniga, nega read va xato tekshiruvlarini inkapsulyatsiya qilish funksiyasini yozmaysiz? Muvaffaqiyatli yoki xato kodini nolga qaytarishi mumkin. Xatolarni qayta ishlash kodi barcha read bloklari orasida umumiy bo'lsa, siz shunday qilishingiz mumkin:

int your_function(whatever) {
    int ret;
    ...

    if ((ret=read_helper(fd, &input_integer)) != 0)
        goto error_case;

    ...

    // Normal return from the function
    return 0;

error_case:
    print_error_message_for_code(ret);
    return ret;
}

read_helper ga qilgan barcha qo'ng'iroqlaringiz o'zlarining qaytish qiymatini ret ga belgilagan ekan, siz butun funksiya davomida bir xil xatolik kodini qayta ishlatishingiz mumkin. Sizning funksiyangiz print_error_message_for_code oddiygina xato kodini kiritish sifatida qabul qiladi va unga mos keladigan xato xabarini chop etish uchun switch yoki massivdan foydalanadi.

Tan olamanki, ko'p odamlar goto dan qo'rqishadi, lekin bir qator ichki bloklar va shart o'zgaruvchilari o'rniga umumiy xatolarni qayta ishlash kodini qayta ishlatish buning uchun to'g'ri foydalanish holati bo'lishi mumkin. Uni toza va o'qilishi mumkin bo'lgan holda saqlang (har bir funktsiya uchun bitta yorliq va xatolarni boshqarish kodini oddiy saqlang).

Bu suiiste'molmi yoki yo'qmi - bu ta'mga bog'liq. Lekin men u bilan ba'zi asosiy muammolarni ko'rmoqdaman

1. hujjatlar butunlay noto'g'ri
2. amalga oshirish if va mumkin bo'lgan osilgan else muammosi tufayli juda xavflidir
3. nomlash bu atrofdagi funktsiyaning dastlabki qaytishi ekanligini bildirmaydi

shuning uchun bu juda yomon uslub.

Agar so'l manba faylida bo'lsa va faqat shu faylda ishlatilsa, men uni biron bir sarlavhada o'chirilganidan ko'ra biroz kamroq haqoratomuz deb bilaman. Lekin menga qaytariladigan makros (ayniqsa, ilovani tugatish uchunhujjatlangan va aslida qaytariladigan) va shartli ravishda qaytariladigan makroni unchalik yoqtirmayman, chunki u yaratishni juda oson qiladi. quyidagi xotira oqishi:

char *buf = malloc(1000);
if (buf == 0) return ENOMEM;

ret_val = read(fd, buf, 1000);
CHECK_READ(ret_val, "buffer read failed")

// do something with buf

free(buf);

Agar men hujjatlarga ishonsam, o'qish muvaffaqiyatsiz tugashi bilan ushbu kod xotirani oqizishiga shubha qilish uchun hech qanday asos yo'q. Hujjatlar to'g'ri bo'lsa ham, men C dagi boshqaruv oqimini yaqqol ko'rishni afzal ko'raman, ya'ni makrolar tomonidan yashirilmaydi. Bundan tashqari, men kodimni tozalash uchun resurslarim bo'lgan holatlar va men yo'q bo'lgan holatlar o'rtasida noaniq izchil ko'rinishini afzal ko'raman, makroni biriga ishlatishdan ko'ra, boshqasi uchun emas. Shunday qilib, so'l mening didim uchun emas, hatto u qat'iy suiiste'mol bo'lmasa ham.

Hujjatlarda aytilgan narsani qilish uchun men shunday bir narsa yozishni afzal ko'raman:

check(retval != -1, "buffer read failed");

check funksiya bo'lgan holda. Yoki assert dan foydalaning. Albatta, agar NDEBUG o'rnatilmagan bo'lsa, tasdiqlash hamma narsani qiladi, shuning uchun agar siz alohida disk raskadrovka va reliz tuzilmalarini rejalashtirmoqchi bo'lsangiz, bu xatolarni qayta ishlash uchun yaxshi emas.

Ibratli amalni bajarish uchun men makrosni butunlay o'tkazib yuborishni afzal ko'raman va yozaman:

if (retval == -1) return DCD_BADREAD;

Bu juda uzoq kod qatori emas va uning turli misollarini birlashtirishdan haqiqiy foyda yo'q. Mavjud, yaxshi hujjatlashtirilgan vositalarni inkapsulyatsiya qilish/yashirish orqali narsalarni yaxshilayman deb o'ylash ham yaxshi fikr emas, bu orqali read xatolarni ko'rsatadi, buni C dasturchilari yaxshi tushunadilar. Boshqa hech narsa bo'lmasa, bu tekshiruv hech qanday sababsiz read siz so'raganingizdan kamroq ma'lumot ishlab chiqarishi mumkinligiga e'tibor bermaydi. Bu to'g'ridan-to'g'ri makroning aybi emas, lekin makroning orqasidagi barcha g'oya noto'g'ri xatolarni tekshirishdan kelib chiqqanga o'xshaydi.

Ehtimol, bu erda nima sodir bo'lgan bo'lsa, ular ilgari tugatilgan makrosga ega edilar, keyin ular muvaffaqiyatsizliklar yuz berganda darhol dasturni to'xtatib qo'ymaslikka qaror qilishdi, buning o'rniga DCD_BADREAD xato kodini qaytarishni xohlashdi. Ular qilishlari kerak bo'lgan narsa makroni butunlay olib tashlash va barcha chaqiruvchi saytlarni o'zgartirish edi - agar siz funktsiyani boshqarish mantiqini o'zgartirsangiz va u qaytaradigan narsani o'zgartirsangiz, bu funktsiyaning manbasini ko'rinadigan darajada o'zgartirgan ma'qul. Buning o'rniga ular xohlagan natijani beradigan minimal mumkin bo'lgan o'zgarishlarni amalga oshirishdi. Ehtimol, o'sha paytda bu yaxshi fikr bo'lib tuyuldi (va sharhlarni yangilashni unutish, afsuski, keng tarqalgan xatodir). Ammo hamma rozi bo'lganidek, bu juda xavfli kodga olib keldi.

Boshqarish oqimini amalga oshiradigan makrolarbo'lishi mumkin suiiste'mol bo'lmasligi mumkin, lekin menimcha, ular faqat boshqaruv oqimiga o'xshab ko'rinadigan va aniq hujjatlashtirilgan holatlarda. Saymon Tathamning birgalikdagi makroslari yodga tushadi - ba'zi odamlar birinchi navbatda birgalikdagi tartiblarni yoqtirmaydilar, lekin siz bu shartni qabul qilsangiz, crReturn deb nomlangan makro hech bo'lmaganda ko'rinadi u qaytib keladiganga o'xshaydi. .

Men so'lni suiiste'mol qilish misolini ikkita sababga ko'ra ko'rib chiqaman: (1) Makrosning nomi nima qilishini aniq ko'rsatmaydi, ayniqsa, u qaytishi mumkin; (2) Makros gapga sintaktik jihatdan ekvivalent emas. Kod kabi

  if (someCondition)
    CHECK_FREAD(whatever);
  else
    do_something_else();

muvaffaqiyatsiz bo'ladi. Men afzal ko'rgan o'zgarish:

#define LOG_AND_RET_ERROR(msg) do {LOG_ERROR(msg); return DCD_BADREAD;} while(0)
  if (x==-1) LOG_AND_RET_ERROR(msg);

Xo'sh, agar siz qisqa funksiyani aniqlasangiz, har bir chaqiruv saytida ko'proq belgilar kiritishingiz kerak. ya'ni

CHECK_FREAD(X, msg)

vs.

if (check_fread(X, msg)) return DCD_BADREAD;

Amalga oshirilgan bo'shliq doirasida u yaxshi bo'lishi mumkin edi (garchi u odatda C++ da norozi bo'lsa ham).

Meni xavotirga soladigan eng yorqin narsa shundaki, ba'zi yangi ishlab chiquvchi buni ko'rib, shunday qilishni o'ylaydi.

if (CHECK_FREAD(...)) {
   /* do stuff here */
} else {
   /* error handle */
}

Bu noto'g'ri ekanligi aniq.

Bundan tashqari, vaziyatni yomonlashtiradigan DCD_BADREAD da iste'mol qilinmasa/kutilgan bo'lmasa, msg ishlatilmaganga o'xshaydi...