[prog.c++] Ну как так то?

Прошу прощения за эмоциональность, но был сегодня сильно удивлен и несколько раздосадован.

Началось все с того, что разыскивая информацию про NAT/STUN/UPnP/port-forwarding и иже с ними, наткнулся на проект pjsip. Был приятно удивлен тому, насколько приятно он оформлен в плане количества документации. Не берусь судить о ее качестве, т.к. смотрел сильно мельком и по диагонали. Но первое впечатление: внушаить!

Под этим положительным впечатлением решил заглянуть в исходники. Ну, думаю, аккуратно написанный на чистом Си проект, надо же поучиться тому, как должен выглядеть хороший Сишный код. Заглянул в первый попавшийся файл:

PJ_DEF(void) pj_stun_auth_cred_dup( pj_pool_t *pool,
                                      pj_stun_auth_cred *dst,
                                      const pj_stun_auth_cred *src)
{
    dst->type = src->type;

    switch (src->type) {
    case PJ_STUN_AUTH_CRED_STATIC:
        pj_strdup(pool, &dst->data.static_cred.realm,
                        &src->data.static_cred.realm);
        pj_strdup(pool, &dst->data.static_cred.username,
                        &src->data.static_cred.username);
        dst->data.static_cred.data_type = src->data.static_cred.data_type;
        pj_strdup(pool, &dst->data.static_cred.data,
                        &src->data.static_cred.data);
        pj_strdup(pool, &dst->data.static_cred.nonce,
                        &src->data.static_cred.nonce);
        break;
    case PJ_STUN_AUTH_CRED_DYNAMIC:
        pj_memcpy(&dst->data.dyn_cred, &src->data.dyn_cred, 
                  sizeof(src->data.dyn_cred));
        break;
    }
}

Так, думаю, неужели у них в проекте pj_strdup ведет к abort если памяти нет?

Заглядываю в потроха pj_strdup, а там ключевой фрагмент выглядит так:

    if (src->slen > 0) {
        dst->ptr = (char*)pj_pool_alloc(pool, src->slen);
        pj_memcpy(dst->ptr, src->ptr, src->slen);
    }

Т.е. дернули пул, получили указатель и скопировали туда значение. Но сам указатель на NULL не проверили. Может он и не может быть NULL?

Что ж, заглянем в pj_pool_alloc:

PJ_IDEF(void*) pj_pool_alloc( pj_pool_t *pool, pj_size_t size)
{
    void *ptr = pj_pool_alloc_from_block(pool->block_list.next, size);
    if (!ptr)
        ptr = pj_pool_allocate_find(pool, size);
    return ptr;
}

Ага, pj_pool_alloc_from_block может вернуть NULL и это контролируется. А что с pj_pool_allocate_find?

А вот pj_pool_allocate_find возвращает NULL, в нескольких местах: тыц и тыц

Т.е., теоритически, память для pj_strdup может быть не найдена. Но pj_strdup это не волнует. Если NULL, то просто попробуем туда записать, а чего париться-то?

Ну вот как так-то?

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

Да и вообще можно расширить: имхо, языки, в которых нет исключений (или хотя бы паник), а есть только коды возврата, которые нужно проверять вручную, не подходят для написания надежного кода. Жирное такое ИМХО, конечно же.

---

А добил меня фрагмент, найденный сегодня же чуть позже в другом проекте. Проект закрытый, так что цинков не будет. Но, образно говоря, там было что-то вроде:

class Handler {
  std::map<Id, std::shared_ptr<Reactor>> reactors_;
  ...
  void terminateTask(Id id) {
    std::shared_ptr<Reactor> reactor;
    auto it = reactors_.find(id);
    if(it != reactors_.end())
      reactor = it->second;
    reactor->stop(); // (1)
    reactor->terminate();
    if(it != reactors_.end())
      reactors_.erase(it);
  }
  ...
};

И вот в точке (1) потенциальный баг, если в terminateTask будет передан id, которого нет в reactors_.

Мне вот сложно понять, как такое вообще оказалось возможно. Ведь если написать так, чтобы этого бага не было, то получится даже компактнее и читабельнее:

class Handler {
  std::map<Id, std::shared_ptr<Reactor>> reactors_;
  ...
  void terminateTask(Id id) {
    auto it = reactors_.find(id);
    if(it != reactors_.end()) {
      it->second->stop(); // (1)
      it->second->terminate();
      reactors_.erase(it);
    }
  }
  ...
};

Ну вот как так-то?

Вопросы, понятное дело, риторические. Errare humanum est