ronantakizawa/codereview-bench huggingface dataset:
Repository: ossrs/srs
File: trunk/src/app/srs_app_rtc_conn.cpp (C++)

Code before:
    trd_ = NULL;

    req_ = NULL;
    source_ = NULL;

    is_started = false;
    session_ = s;

    mw_msgs = 0;
    realtime = true;

    nack_enabled_ = false;
    nack_no_copy_ = false;

    _srs_config->subscribe(this);
    nack_epp = new SrsErrorPithyPrint();
    pli_worker_ = new SrsRtcPLIWorker(this);

    cache_ssrc0_ = cache_ssrc1_ = cache_ssrc2_ = 0;
    cache_track0_ = cache_track1_ = cache_track2_ = NULL;
}

SrsRtcPlayStream::~SrsRtcPlayStream()
{
    // TODO: FIXME: Should not do callback in de-constructor?
    if (_srs_rtc_hijacker) {
        _srs_rtc_hijacker->on_stop_play(session_, this, req_);
    }

    _srs_config->unsubscribe(this);

    srs_freep(nack_epp);
    srs_freep(pli_worker_);
    srs_freep(trd_);
    srs_freep(req_);

    if (true) {
        std::map<uint32_t, SrsRtcAudioSendTrack*>::iterator it;
        for (it = audio_tracks_.begin(); it != audio_tracks_.end(); ++it) {
            srs_freep(it->second);
        }
    }

    if (true) {
        std::map<uint32_t, SrsRtcVideoSendTrack*>::iterator it;
        for (it = video_tracks_.begin(); it != video_tracks_.end(); ++it) {
            srs_freep(it->second);
        }
    }
	
    // update the statistic when client coveried.

Review comment (security):
This is the issue that Guanghua mentioned before, regarding the problem of calling HTTP hooks in the destructor function.

publish' and 'play' must be called synchronously because they have authentication functionality.

unpublish' and 'stop' definitely do not require authentication, so it is possible to call them asynchronously.

Refer to `ISrsAsyncCallTask`.

Calling HTTP in the destructor can lead to coroutine switching, which carries a high risk.

`TRANS_BY_GPT3`

Code after:
    trd_ = NULL;

    req_ = NULL;
    source_ = NULL;

    is_started = false;
    session_ = s;

    mw_msgs = 0;
    realtime = true;

    nack_enabled_ = false;
    nack_no_copy_ = false;

    _srs_config->subscribe(this);
    nack_epp = new SrsErrorPithyPrint();
    pli_worker_ = new SrsRtcPLIWorker(this);

    cache_ssrc0_ = cache_ssrc1_ = cache_ssrc2_ = 0;
    cache_track0_ = cache_track1_ = cache_track2_ = NULL;
}

SrsRtcPlayStream::~SrsRtcPlayStream()
{
    if (req_) {
        http_hooks_on_stop();		
    }

    // TODO: FIXME: Should not do callback in de-constructor?
    if (_srs_rtc_hijacker) {
        _srs_rtc_hijacker->on_stop_play(session_, this, req_);
    }

    _srs_config->unsubscribe(this);

    srs_freep(nack_epp);
    srs_freep(pli_worker_);
    srs_freep(trd_);
    srs_freep(req_);

    if (true) {
        std::map<uint32_t, SrsRtcAudioSendTrack*>::iterator it;
        for (it = audio_tracks_.begin(); it != audio_tracks_.end(); ++it) {
            srs_freep(it->second);
        }
    }

    if (true) {
        std::map<uint32_t, SrsRtcVideoSendTrack*>::iterator it;
        for (it = video_tracks_.begin(); it != video_tracks_.end(); ++it) {
            srs_freep(it->second);
Repository: ossrs/srs
File: trunk/src/app/srs_app_http_static.cpp (C++)

Code before:
        hooks = conf->args;
    }

    for (int i = 0; i < (int)hooks.size(); i++) {
        std::string url = hooks.at(i);
        SrsHttpHooks::on_stop(url, req);
    }

    return;
}

srs_error_t SrsVodStream::on_timer(srs_utime_t interval)
{
    srs_error_t err = srs_success;

    std::map<std::string, srs_utime_t>::iterator it;
    for (it = map_secret_validity_.begin(); it != map_secret_validity_.end(); ++it) {
        string secret = it->first;
        SrsRequest* req = map_secret_req_[secret];
        srs_utime_t hls_window = _srs_config->get_hls_window(req->vhost);
        if (it->second + (2 * hls_window) < srs_get_system_time()) {
            http_hooks_on_stop(req);
            srs_freep(req);
            map_secret_req_.erase(secret);

            SrsStatistic* stat = SrsStatistic::instance();
            stat->on_disconnect(secret);
            map_secret_validity_.erase(it);

            break;
        }
    }

    return err;
}

SrsHttpStaticServer::SrsHttpStaticServer(SrsServer* svr)
{
    server = svr;
    _srs_config->subscribe(this);
}

SrsHttpStaticServer::~SrsHttpStaticServer()
{
    _srs_config->unsubscribe(this);
}

srs_error_t SrsHttpStaticServer::initialize()
{
    srs_error_t err = srs_success;
    

Review comment (security):
map_secret_validity_ Manage whether it is expired,
map_secret_req_ Match secret and req
Here's a suggestion, you can try using only one map to manage, and put req and timeout in one struct.
std::map<std::string, struct SrsM3u8SecretInfo> map_secret_info
The advantage of this is that you don't have to manage two maps and make multiple correspondences. If it times out, it will definitely be cleared.

`TRANS_BY_GPT3`

Code after:
    // @see https://github.com/ossrs/srs/issues/475
    vector<string> hooks;

    if (true) {
        SrsConfDirective* conf = _srs_config->get_vhost_on_stop(req->vhost);

        if (!conf) {
            srs_info("ignore the empty http callback: on_stop");
            return;
        }

        hooks = conf->args;
    }

    for (int i = 0; i < (int)hooks.size(); i++) {
        std::string url = hooks.at(i);
        SrsHttpHooks::on_stop(url, req);
    }

    return;
}

srs_error_t SrsVodStream::on_timer(srs_utime_t interval)
{
    srs_error_t err = srs_success;

    std::map<std::string, SrsM3u8CtxInfo>::iterator it;
    for (it = map_ctx_info_.begin(); it != map_ctx_info_.end(); ++it) {
        string ctx = it->first;
        SrsRequest* req = it->second.req;
        srs_utime_t hls_window = _srs_config->get_hls_window(req->vhost);
        if (it->second.request_time + (2 * hls_window) < srs_get_system_time()) {
            http_hooks_on_stop(req);
            srs_freep(req);

            SrsStatistic* stat = SrsStatistic::instance();
            stat->on_disconnect(ctx);
            map_ctx_info_.erase(it);

            break;
        }
    }

    return err;
}

SrsHttpStaticServer::SrsHttpStaticServer(SrsServer* svr)
{
    server = svr;
    _srs_config->subscribe(this);
}
Repository: ossrs/srs
File: trunk/src/srt/srt_to_rtmp.cpp (C++)

Code before:
//
// Copyright (c) 2013-2021 Runner365
//
// SPDX-License-Identifier: MIT
//

#include "srt_to_rtmp.hpp"
#include "stringex.hpp"
#include "time_help.h"
#include <srs_kernel_log.hpp>
#include <srs_kernel_error.hpp>
#include <srs_kernel_buffer.hpp>
#include <srs_kernel_utility.hpp>
#include <srs_app_rtmp_conn.hpp>
#include <srs_app_config.hpp>
#include <srs_kernel_stream.hpp>
#include <list>

std::shared_ptr<srt2rtmp> srt2rtmp::s_srt2rtmp_ptr;

std::shared_ptr<srt2rtmp> srt2rtmp::get_instance() {
    if (!s_srt2rtmp_ptr) {
        s_srt2rtmp_ptr = std::make_shared<srt2rtmp>();
    }
    return s_srt2rtmp_ptr;
}

srt2rtmp::srt2rtmp():_lastcheck_ts(0) {

}

srt2rtmp::~srt2rtmp() {
    release();
}

srs_error_t srt2rtmp::init() {
    srs_error_t err = srs_success;

    if (_trd_ptr.get() != nullptr) {
        return srs_error_wrap(err, "don't start thread again");
    }

    _trd_ptr = std::make_shared<SrsSTCoroutine>("srt2rtmp", this);

Review comment (suggestion):
The `#include <sstream>` has already been imported in the `srs_app_config.hpp` header file, so there is no need to repeat `#include <sstream>`.

`TRANS_BY_GPT3`

Code after:
//
// Copyright (c) 2013-2021 Runner365
//
// SPDX-License-Identifier: MIT
//

#include "srt_to_rtmp.hpp"
#include "stringex.hpp"
#include "time_help.h"
#include <srs_kernel_log.hpp>
#include <srs_kernel_error.hpp>
#include <srs_kernel_buffer.hpp>
#include <srs_kernel_utility.hpp>
#include <srs_app_rtmp_conn.hpp>
#include <srs_app_config.hpp>
#include <srs_kernel_stream.hpp>
#include <list>
#include <sstream>

std::shared_ptr<srt2rtmp> srt2rtmp::s_srt2rtmp_ptr;

std::shared_ptr<srt2rtmp> srt2rtmp::get_instance() {
    if (!s_srt2rtmp_ptr) {
        s_srt2rtmp_ptr = std::make_shared<srt2rtmp>();
    }
    return s_srt2rtmp_ptr;
}

srt2rtmp::srt2rtmp():_lastcheck_ts(0) {

}

srt2rtmp::~srt2rtmp() {
    release();
}

srs_error_t srt2rtmp::init() {
    srs_error_t err = srs_success;

    if (_trd_ptr.get() != nullptr) {
        return srs_error_wrap(err, "don't start thread again");
    }
Repository: ossrs/srs
File: trunk/src/app/srs_app_config.cpp (C++)

Code before:
    }
    
    if ((err = parse_buffer(&buffer)) != srs_success) {
        return srs_error_wrap(err, "parse buffer");
    }
    
    return err;
}

srs_error_t SrsConfig::parse_include_file(const char *filename)
{
    srs_error_t err = srs_success;

    std::string include_file = filename;

    if (include_file.empty()) {
        return srs_error_new(ERROR_SYSTEM_CONFIG_INVALID, "empty include config");
    }

    SrsConfigBuffer buffer;

    if ((err = buffer.fullfill(include_file.c_str())) != srs_success) {
        return srs_error_wrap(err, "buffer fullfil");
    }

    if ((err = parse_include_buffer(&buffer)) != srs_success) {
        return srs_error_wrap(err, "parse include buffer");
    }

    return err;
}
// LCOV_EXCL_STOP

srs_error_t SrsConfig::check_config()
{
    srs_error_t err = srs_success;
    
    if ((err = check_normal_config()) != srs_success) {
        return srs_error_wrap(err, "check normal");
    }
    
    if ((err = check_number_connections()) != srs_success) {
        return srs_error_wrap(err, "check connections");
    }

    // If use the full.conf, fail.
    if (is_full_config()) {
        return srs_error_new(ERROR_SYSTEM_CONFIG_INVALID,
            "never use full.conf(%s)", config_file.c_str());
    }
    

Review comment (suggestion):
This function does not need to be encapsulated once, it can be called directly because it is only called in this one place.

`TRANS_BY_GPT3`

Code after:
    
    if ((err = parse_buffer(buffer)) != srs_success) {
        return srs_error_wrap(err, "parse buffer");
    }
    
    return err;
}

srs_error_t SrsConfig::build_buffer(string src, SrsConfigBuffer** pbuffer)
{
    srs_error_t err = srs_success;

    SrsConfigBuffer* buffer = new SrsConfigBuffer();

    if ((err = buffer->fullfill(src.c_str())) != srs_success) {
        srs_freep(buffer);
        return srs_error_wrap(err, "read from src %s", src.c_str());
    }

    *pbuffer = buffer;
    return err;
}
// LCOV_EXCL_STOP

srs_error_t SrsConfig::check_config()
{
    srs_error_t err = srs_success;
    
    if ((err = check_normal_config()) != srs_success) {
        return srs_error_wrap(err, "check normal");
    }
    
    if ((err = check_number_connections()) != srs_success) {
        return srs_error_wrap(err, "check connections");
    }

    // If use the full.conf, fail.
    if (is_full_config()) {
        return srs_error_new(ERROR_SYSTEM_CONFIG_INVALID,
            "never use full.conf(%s)", config_file.c_str());
    }
    
    return err;
}

srs_error_t SrsConfig::check_normal_config()
{
    srs_error_t err = srs_success;
    
    srs_trace("srs checking config...");
Repository: ossrs/srs
File: trunk/src/app/srs_app_config.cpp (C++)

Code before:
    // mock by dolphin mode.
    // for the dolphin will start srs with specified params.
    if (dolphin) {
        // for RTMP.
        set_config_directive(root, "listen", dolphin_rtmp_port);
        
        // for HTTP
        set_config_directive(root, "http_server", "");
        SrsConfDirective* http_server = root->get("http_server");
        set_config_directive(http_server, "enabled", "on");
        set_config_directive(http_server, "listen", dolphin_http_port);
        
        // others.
        set_config_directive(root, "daemon", "off");
        set_config_directive(root, "srs_log_tank", "console");
    }
    
    return err;
}

srs_error_t SrsConfig::parse_include_buffer(SrsConfigBuffer *buffer)
{
    srs_error_t err = srs_success;

    // Parse root tree from buffer.
    if ((err = root->parse(buffer, this)) != srs_success) {
        return srs_error_wrap(err, "root parse");
    }

    return err;
}

string SrsConfig::cwd()
{
    return _cwd;
}

string SrsConfig::argv()
{
    return _argv;
}

bool SrsConfig::get_daemon()
{
    SrsConfDirective* conf = root->get("daemon");
    if (!conf || conf->arg0().empty()) {
        return true;
    }
    
    return SRS_CONF_PERFER_TRUE(conf->arg0());
}

Review comment (suggestion):
Does 'include' only support top-level ones, such as: 

```
listen 1935;
include rtmp.conf rtc.conf;
```

And does not support this kind:

```
listen 1935;
rtc_server {
  include rtc.conf;
}
```

It seems that the sub-file is being parsed using root. However, `SrsConfDirective::parse_conf` can be recursively called, which can cause issues in the later cases. It would be helpful to test it using utest.

`TRANS_BY_GPT3`

Code after:
        set_config_directive(http_server, "enabled", "on");
        set_config_directive(http_server, "listen", dolphin_http_port);
        
        // others.
        set_config_directive(root, "daemon", "off");
        set_config_directive(root, "srs_log_tank", "console");
    }
    
    return err;
}

string SrsConfig::cwd()
{
    return _cwd;
}

string SrsConfig::argv()
{
    return _argv;
}

bool SrsConfig::get_daemon()
{
    SrsConfDirective* conf = root->get("daemon");
    if (!conf || conf->arg0().empty()) {
        return true;
    }
    
    return SRS_CONF_PERFER_TRUE(conf->arg0());
}

bool SrsConfig::get_in_docker()
{
    static bool DEFAULT = false;

    SrsConfDirective* conf = root->get("in_docker");
    if (!conf) {
        return DEFAULT;
    }

    return SRS_CONF_PERFER_FALSE(conf->arg0());
}

bool SrsConfig::is_full_config()
{
    static bool DEFAULT = false;

    SrsConfDirective* conf = root->get("is_full");
    if (!conf) {
        return DEFAULT;
    }
Repository: ossrs/srs
File: trunk/src/srt/srt_to_rtmp.cpp (C++)

Code before:
}

int rtmp_client::on_data_callback(SRT_DATA_MSG_PTR data_ptr, unsigned int media_type,
                                uint64_t dts, uint64_t pts)
{
    srs_error_t err = srs_success;
    if (!data_ptr || (data_ptr->get_data() == nullptr) || (data_ptr->data_len() == 0)) {
        assert(0);
        return 0;
    }

    auto avs_ptr = std::make_shared<SrsBuffer>((char*)data_ptr->get_data(), data_ptr->data_len());

    if (media_type == STREAM_TYPE_VIDEO_H264) {
        err = on_ts_video(avs_ptr, dts, pts);
    } else if (media_type == STREAM_TYPE_AUDIO_AAC) {
        err = on_ts_audio(avs_ptr, dts, pts);
    } else {
        srs_error("mpegts demux unkown stream type:0x%02x, only support h264+aac", media_type);
        return 0;
    }

    if (err != srs_success) {
        srs_error("send media data error:%s", srs_error_desc(err).c_str());
        srs_freep(err);
        return -1;
    }
    return 0;
}

rtmp_packet_queue::rtmp_packet_queue():_queue_timeout(QUEUE_DEF_TIMEOUT)
    ,_queue_maxlen(QUEUE_LEN_MAX)
    ,_first_packet_t(-1)
    ,_first_local_t(-1) {

}

rtmp_packet_queue::~rtmp_packet_queue() {
    for (auto item : _send_map) {
        rtmp_packet_info_s info = item.second;
        if (info._data) {
            delete info._data;
        }
    }
    _send_map.clear();
}

void rtmp_packet_queue::set_queue_timeout(int64_t queue_timeout) {
    _queue_timeout = queue_timeout;
}


Review comment (suggestion):
`srs_error_code`

```
if (err != srs_success) {
    return srs_error_wrap(err, "send media data error");
}
```

Code after:
    }
    return err;
}

int rtmp_client::on_data_callback(SRT_DATA_MSG_PTR data_ptr, unsigned int media_type,
                                uint64_t dts, uint64_t pts)
{
    srs_error_t err = srs_success;
    if (!data_ptr || (data_ptr->get_data() == nullptr) || (data_ptr->data_len() == 0)) {
        assert(0);
        return 0;
    }

    auto avs_ptr = std::make_shared<SrsBuffer>((char*)data_ptr->get_data(), data_ptr->data_len());

    if (media_type == STREAM_TYPE_VIDEO_H264) {
        err = on_ts_video(avs_ptr, dts, pts);
    } else if (media_type == STREAM_TYPE_AUDIO_AAC) {
        err = on_ts_audio(avs_ptr, dts, pts);
    } else {
        srs_error("mpegts demux unkown stream type:0x%02x, only support h264+aac", media_type);
        return 0;
    }

    if (err != srs_success) {
        srs_error("send media data error:%s", srs_error_desc(err).c_str());
        int err_code = srs_error_code(err);
        srs_freep(err);
        return err_code;
    }
    return 0;
}

rtmp_packet_queue::rtmp_packet_queue():_queue_timeout(QUEUE_DEF_TIMEOUT)
    ,_queue_maxlen(QUEUE_LEN_MAX)
    ,_first_packet_t(-1)
    ,_first_local_t(-1) {

}

rtmp_packet_queue::~rtmp_packet_queue() {
    for (auto item : _send_map) {
        rtmp_packet_info_s info = item.second;
        if (info._data) {
            delete info._data;
        }
    }
    _send_map.clear();
}

void rtmp_packet_queue::set_queue_timeout(int64_t queue_timeout) {
Repository: ossrs/srs
File: trunk/src/kernel/srs_kernel_error.cpp (C++)

Code before:
        next = this;
        while (next) {
            ss << "thread [" << getpid() << "][" << next->cid.c_str() << "]: "
            << next->func << "() [" << next->file << ":" << next->line << "]"
            << "[errno=" << next->rerrno << "]";

            next = next->wrapped;

            if (next) {
                ss << endl;
            }
        }

        desc = ss.str();
    }

    return desc;
}

std::string SrsCplxError::summary() {
    if (_summary.empty()) {
        stringstream ss;

        SrsCplxError* next = this;
        while (next) {
            if (!next->wrapped) {
                ss << next->msg;
            }
            next = next->wrapped;
        }

        _summary = ss.str();
    }

    return _summary;
}

SrsCplxError* SrsCplxError::create(const char* func, const char* file, int line, int code, const char* fmt, ...) {
    int rerrno = (int)errno;

    va_list ap;
    va_start(ap, fmt);
    static char buffer[4096];
    vsnprintf(buffer, sizeof(buffer), fmt, ap);
    va_end(ap);
    
    SrsCplxError* err = new SrsCplxError();
    
    err->func = func;
    err->file = file;
    err->line = line;

Review comment (suggestion):
Do not make changes in a PR that are unrelated to yourself. This modification is for the public Error module and has nothing to do with the configuration file parsing.

`TRANS_BY_GPT3`

Code after:
        next = this;
        while (next) {
            ss << "thread [" << getpid() << "][" << next->cid.c_str() << "]: "
            << next->func << "() [" << next->file << ":" << next->line << "]"
            << "[errno=" << next->rerrno << "]";

            next = next->wrapped;

            if (next) {
                ss << endl;
            }
        }

        desc = ss.str();
    }

    return desc;
}

std::string SrsCplxError::summary() {
    if (_summary.empty()) {
        stringstream ss;

        SrsCplxError* next = this;
        while (next) {
            ss << " : " << next->msg;
            next = next->wrapped;
        }

        _summary = ss.str();
    }

    return _summary;
}

SrsCplxError* SrsCplxError::create(const char* func, const char* file, int line, int code, const char* fmt, ...) {
    int rerrno = (int)errno;

    va_list ap;
    va_start(ap, fmt);
    static char buffer[4096];
    vsnprintf(buffer, sizeof(buffer), fmt, ap);
    va_end(ap);
    
    SrsCplxError* err = new SrsCplxError();
    
    err->func = func;
    err->file = file;
    err->line = line;
    err->code = code;
    err->rerrno = rerrno;
Repository: ossrs/srs
File: trunk/src/app/srs_app_source.cpp (C++)

Code before:
{
    // unknown state?
    if (die_at == 0) {
        return false;
    }
    
    // still publishing?
    if (!_can_publish || !publish_edge->can_publish()) {
        return false;
    }
    
    // has any consumers?
    if (!consumers.empty()) {
        return false;
    }
    
    srs_utime_t now = srs_get_system_time();
    if (now > die_at + SRS_SOURCE_CLEANUP) {
        return true;
    }
    
    return false;
}

#ifdef SRS_PERF_KICKOFF_AS_NO_ONE_WATCHING
bool SrsLiveSource::expired_as_no_one_watching(srs_utime_t timeout)
{
    if (no_one_wating_at == 0) {
        return false;
    }
    srs_utime_t now = srs_get_system_time();
    if (now > no_one_wating_at + timeout) {
        return true;
    }
    return false;
}
#endif

srs_error_t SrsLiveSource::initialize(SrsRequest* r, ISrsLiveSourceHandler* h)
{
    srs_error_t err = srs_success;
    
    srs_assert(h);
    srs_assert(!req);
    
    handler = h;
    req = r->copy();
    atc = _srs_config->get_atc(req->vhost);
    
    if ((err = hub->initialize(this, req)) != srs_success) {
        return srs_error_wrap(err, "hub");

Review comment (suggestion):
This function name can be changed to:

```cpp
bool SrsLiveSource::idle_for(srs_utime_t timeout)
```

Please remove `SRS_PERF_KICKOFF_AS_NO_ONE_WATCHING`, there is no need for macro definition.

`TRANS_BY_GPT3`

Code after:
bool SrsLiveSource::stream_is_dead()
{
    // unknown state?
    if (stream_die_at_ == 0) {
        return false;
    }
    
    // still publishing?
    if (!_can_publish || !publish_edge->can_publish()) {
        return false;
    }
    
    // has any consumers?
    if (!consumers.empty()) {
        return false;
    }
    
    srs_utime_t now = srs_get_system_time();
    if (now > stream_die_at_ + SRS_SOURCE_CLEANUP) {
        return true;
    }
    
    return false;
}

bool SrsLiveSource::publisher_is_idle_for(srs_utime_t timeout)
{
    if (!publisher_idle_at_ || !timeout) {
        return false;
    }

    srs_utime_t now = srs_get_system_time();
    if (now > publisher_idle_at_ + timeout) {
        return true;
    }
    return false;
}

srs_error_t SrsLiveSource::initialize(SrsRequest* r, ISrsLiveSourceHandler* h)
{
    srs_error_t err = srs_success;
    
    srs_assert(h);
    srs_assert(!req);
    
    handler = h;
    req = r->copy();
    atc = _srs_config->get_atc(req->vhost);

    if ((err = format_->initialize()) != srs_success) {
        return srs_error_wrap(err, "format initialize");