ronantakizawa/codereview-bench huggingface dataset:
Repository: fyne-io/fyne
File: resource.go (Go)

Code before:
	return NewStaticResource(name, bytes), nil
}

// LoadResourceFromURLString creates a new [StaticResource] in memory using the body of the specified URL.
func LoadResourceFromURLString(urlStr string) (Resource, error) {
	res, err := http.Get(urlStr)
	if err != nil {
		return nil, err
	}
	defer res.Body.Close()

	bytes, err := io.ReadAll(res.Body)
	if err != nil {
		return nil, err
	}

	name := filepath.Base(urlStr)
	return NewStaticResource(name, bytes), nil
}

// CacheResourceFromURLString creates a new [StaticResource] in memory using the body of the specified URL.
// It also adds the bytes to our cache so that future requests for this icon will ready locally instead of the web server.
//
// Since: 2.8
func CacheResourceFromURLString(urlStr string) (Resource, error) {
	cache := CurrentApp().Cache()
	if data, err := cache.Get(urlStr); err == nil {
		name := filepath.Base(urlStr)
		return NewStaticResource(name, data), nil
	}

	res, err := LoadResourceFromURLString(urlStr)
	if err != nil {
		return nil, err
	}

	_ = cache.Set(urlStr, res.Content())
	return res, nil
}


Review comment (suggestion):
Just a note: I think a better signature for this function would be to take either a `*url.URL` or `fyne.URI` as a param, but since we already have `LoadResourceFromURLString` we need to follow the same pattern. Could be another thing to add to the list of potential 3.0 changes

Code after:
	return NewStaticResource(name, bytes), nil
}

// LoadResourceFromURLString creates a new [StaticResource] in memory using the body of the specified URL.
func LoadResourceFromURLString(urlStr string) (Resource, error) {
	res, err := http.Get(urlStr)
	if err != nil {
		return nil, err
	}
	defer res.Body.Close()

	bytes, err := io.ReadAll(res.Body)
	if err != nil {
		return nil, err
	}

	name := filepath.Base(urlStr)
	return NewStaticResource(name, bytes), nil
}

// CacheResourceFromURLString creates a new [StaticResource] in memory using the body of the specified URL.
// It also adds the content to our cache so that future requests for this resource will be available locally
// instead of downloaded from the URL.
//
// Since: 2.8
func CacheResourceFromURLString(urlStr string) (Resource, error) {
	cache := CurrentApp().Cache()
	if read, err := cache.Read(urlStr); err == nil {
		name := filepath.Base(urlStr)

		data, err := io.ReadAll(read)
		_ = read.Close()
		if err != nil {
			return nil, err
		}
		return NewStaticResource(name, data), nil
	}

	res, err := LoadResourceFromURLString(urlStr)
	if err != nil {
		return nil, err
	}

	write, _ := cache.Write(urlStr)
	defer write.Close()
	data := res.Content()

	n, err := write.Write(data)
	for n < len(data) {
		if err != nil {
			return res, err
Repository: fyne-io/fyne
File: resource.go (Go)

Code before:
		return nil, err
	}

	name := filepath.Base(path)
	return NewStaticResource(name, bytes), nil
}

// LoadResourceFromURLString creates a new [StaticResource] in memory using the body of the specified URL.
func LoadResourceFromURLString(urlStr string) (Resource, error) {
	res, err := http.Get(urlStr)
	if err != nil {
		return nil, err
	}
	defer res.Body.Close()

	bytes, err := io.ReadAll(res.Body)
	if err != nil {
		return nil, err
	}

	name := filepath.Base(urlStr)
	return NewStaticResource(name, bytes), nil
}

// CacheResourceFromURLString creates a new [StaticResource] in memory using the body of the specified URL.
// It also adds the bytes to our cache so that future requests for this icon will ready locally instead of the web server.
//
// Since: 2.8
func CacheResourceFromURLString(urlStr string) (Resource, error) {
	cache := CurrentApp().Cache()
	if read, err := cache.Read(urlStr); err == nil {
		name := filepath.Base(urlStr)

		data, err := io.ReadAll(read)
		_ = read.Close()
		if err != nil {
			return nil, err
		}
		return NewStaticResource(name, data), nil
	}

	res, err := LoadResourceFromURLString(urlStr)
	if err != nil {
		return nil, err
	}

	write, _ := cache.Write(urlStr)
	defer write.Close()
	data := res.Content()

	n, err := write.Write(data)

Review comment (performance):
```suggestion
// It also adds the content to our cache so that future requests for this resource will be available locally instead of from the URL.
```

Code after:
		return nil, err
	}

	name := filepath.Base(path)
	return NewStaticResource(name, bytes), nil
}

// LoadResourceFromURLString creates a new [StaticResource] in memory using the body of the specified URL.
func LoadResourceFromURLString(urlStr string) (Resource, error) {
	res, err := http.Get(urlStr)
	if err != nil {
		return nil, err
	}
	defer res.Body.Close()

	bytes, err := io.ReadAll(res.Body)
	if err != nil {
		return nil, err
	}

	name := filepath.Base(urlStr)
	return NewStaticResource(name, bytes), nil
}

// CacheResourceFromURLString creates a new [StaticResource] in memory using the body of the specified URL.
// It also adds the content to our cache so that future requests for this resource will be available locally
// instead of downloaded from the URL.
//
// Since: 2.8
func CacheResourceFromURLString(urlStr string) (Resource, error) {
	cache := CurrentApp().Cache()
	if read, err := cache.Read(urlStr); err == nil {
		name := filepath.Base(urlStr)

		data, err := io.ReadAll(read)
		_ = read.Close()
		if err != nil {
			return nil, err
		}
		return NewStaticResource(name, data), nil
	}

	res, err := LoadResourceFromURLString(urlStr)
	if err != nil {
		return nil, err
	}

	write, _ := cache.Write(urlStr)
	defer write.Close()
	data := res.Content()
Repository: gabime/spdlog
File: include/spdlog/details/logger_impl.h (C/C++)

Code before:
        details::log_msg log_msg(&_name, lvl);
        log_msg.raw.write(fmt, args...);
        if (_formatter->do_msg_count()) {
            log_msg.msg_id = _msg_counter.fetch_add(1, std::memory_order_relaxed);
        }
        _sink_it(log_msg);
    }
    catch (const std::exception &ex)
    {
        _err_handler(ex.what());
    }
    catch (...)
    {
        _err_handler("Unknown exception");
    }
}

template <typename... Args>
inline void spdlog::logger::log(level::level_enum lvl, const char* msg)
{
    if (!should_log(lvl)) return;
    try
    {
        details::log_msg log_msg(&_name, lvl);
        log_msg.raw << msg;
        if (_formatter->do_msg_count()) {
            log_msg.msg_id = _msg_counter.fetch_add(1, std::memory_order_relaxed);
        }
        _sink_it(log_msg);
    }
    catch (const std::exception &ex)
    {
        _err_handler(ex.what());
    }
    catch (...)
    {
        _err_handler("Unknown exception");
    }

}

template<typename T>
inline void spdlog::logger::log(level::level_enum lvl, const T& msg)
{
    if (!should_log(lvl)) return;
    try
    {
        details::log_msg log_msg(&_name, lvl);
        log_msg.raw << msg;
        if (_formatter->do_msg_count()) {
            log_msg.msg_id = _msg_counter.fetch_add(1, std::memory_order_relaxed);

Review comment (suggestion):
This is nice.. (though I wonder if the cost of calling ```do_msg_count()``` is similar or to the atomic increment  though..)

Code after:
        details::log_msg log_msg(&_name, lvl);
        log_msg.raw.write(fmt, args...);

#if defined(SPDLOG_ENABLE_MESSAGE_COUNTER)
        log_msg.msg_id = _msg_counter.fetch_add(1, std::memory_order_relaxed);
#endif

        _sink_it(log_msg);
    }
    catch (const std::exception &ex)
    {
        _err_handler(ex.what());
    }
    catch (...)
    {
        _err_handler("Unknown exception");
    }
}

template <typename... Args>
inline void spdlog::logger::log(level::level_enum lvl, const char* msg)
{
    if (!should_log(lvl)) return;
    try
    {
        details::log_msg log_msg(&_name, lvl);
        log_msg.raw << msg;

#if defined(SPDLOG_ENABLE_MESSAGE_COUNTER)
        log_msg.msg_id = _msg_counter.fetch_add(1, std::memory_order_relaxed);
#endif

        _sink_it(log_msg);
    }
    catch (const std::exception &ex)
    {
        _err_handler(ex.what());
    }
    catch (...)
    {
        _err_handler("Unknown exception");
    }

}

template<typename T>
inline void spdlog::logger::log(level::level_enum lvl, const T& msg)
{
    if (!should_log(lvl)) return;
    try
    {
Repository: gabime/spdlog
File: include/spdlog/sinks/systemd_sink.h (C/C++)

Code before:
}

/**
 * Sink that write to systemd using the `sd_journal_print()` library call.
 *
 * Locking is not needed, as `sd_journal_print()` itself is thread-safe.
 */
template<typename Mutex>
class systemd_sink : public base_sink<Mutex>
{
public:
    //
    explicit systemd_sink(void) {}

    ~systemd_sink() override {}

    systemd_sink(const systemd_sink &) = delete;
    systemd_sink &operator=(const systemd_sink &) = delete;

protected:
    void sink_it_(const details::log_msg &msg) override
    {
        if( sd_journal_print(
                syslog_level(msg.level),
                "%s",
                fmt::to_string(msg.payload).c_str()
            )
        )
            throw spdlog_ex("Failed writing to systemd");
    }

    void flush_() override {}
};

using systemd_sink_mt = systemd_sink<std::mutex>;
using systemd_sink_st = systemd_sink<details::null_mutex>;
} // namespace sinks

// Create and register a syslog logger
template<typename Factory = default_factory>
inline std::shared_ptr<logger> systemd_logger_mt(
    const std::string &logger_name)
{
    return Factory::template create<sinks::systemd_sink_mt>(logger_name);
}

template<typename Factory = default_factory>
inline std::shared_ptr<logger> systemd_logger_st(
    const std::string &logger_name)
{
    return Factory::template create<sinks::systemd_sink_st>(logger_name);

Review comment (performance):
fmt::to_string cretes a new string copy which should be avoided  in this hot path. Is there another way here?

Code after:
}

/**
 * Sink that write to systemd using the `sd_journal_print()` library call.
 *
 * Locking is not needed, as `sd_journal_print()` itself is thread-safe.
 */
template<typename Mutex>
class systemd_sink : public base_sink<Mutex>
{
public:
    //
    explicit systemd_sink(void) {}

    ~systemd_sink() override {}

    systemd_sink(const systemd_sink &) = delete;
    systemd_sink &operator=(const systemd_sink &) = delete;

protected:
    void sink_it_(const details::log_msg &msg) override
    {
        if( sd_journal_print(
                syslog_level(msg.level),
                "%.*s",
                static_cast<int>(msg.payload.size()),
                msg.payload.data()
            )
        )
            throw spdlog_ex("Failed writing to systemd");
    }

    void flush_() override {}
};

using systemd_sink_mt = systemd_sink<std::mutex>;
using systemd_sink_st = systemd_sink<details::null_mutex>;
} // namespace sinks

// Create and register a syslog logger
template<typename Factory = default_factory>
inline std::shared_ptr<logger> systemd_logger_mt(
    const std::string &logger_name)
{
    return Factory::template create<sinks::systemd_sink_mt>(logger_name);
}

template<typename Factory = default_factory>
inline std::shared_ptr<logger> systemd_logger_st(
    const std::string &logger_name)
{
Repository: gabime/spdlog
File: include/spdlog/sinks/ringbuffer_sink-inl.h (C/C++)

Code before:
// Copyright(c) 2015-present, Gabi Melman & spdlog contributors.
// Distributed under the MIT License (http://opensource.org/licenses/MIT)

#pragma once

#ifndef SPDLOG_HEADER_ONLY
#include "spdlog/sinks/ringbuffer_sink.h"
#endif

#include "spdlog/common.h"
#include "spdlog/details/os.h"

namespace spdlog {
namespace sinks {

template<typename Mutex>
SPDLOG_INLINE ringbuffer_sink<Mutex>::ringbuffer_sink(size_t buf_size)
{
    buf=details::circular_q<std::string>(buf_size);
}

template<typename Mutex>
SPDLOG_INLINE void ringbuffer_sink<Mutex>::sink_it_(const details::log_msg &msg)
{
    memory_buf_t formatted;
    base_sink<Mutex>::formatter_->format(msg, formatted);
    buf.push_back(fmt::to_string(formatted));
}

template<typename Mutex>
SPDLOG_INLINE std::vector<std::string> ringbuffer_sink<Mutex>::last(size_t lim)
{
    std::lock_guard<Mutex> lock(base_sink<Mutex>::mutex_);
    std::vector<std::string> ret;
    ret.reserve(lim);
    size_t num=0;
    for(size_t i=0; i<buf.size(); i++){
        num++;
        ret.push_back(buf.at(i));
        if(lim>0 && num==lim) break;
    }
    return ret;
}


Review comment (performance):
Using circular_q<log_msg_buffer> instead of string will improve performance:
1: Avoids allocation of string in each message
2. format() can be avoided until really needed (in the last(size_t lim) method)
 see https://github.com/gabime/spdlog/blob/v1.x/include/spdlog/details/backtracer.h and https://github.com/gabime/spdlog/blob/v1.x/include/spdlog/details/backtracer-inl.h   as an example



Code after:
// Copyright(c) 2015-present, Gabi Melman & spdlog contributors.
// Distributed under the MIT License (http://opensource.org/licenses/MIT)

#pragma once

#ifndef SPDLOG_HEADER_ONLY
#include "spdlog/sinks/ringbuffer_sink.h"
#endif

#include "spdlog/common.h"
#include "spdlog/details/os.h"

namespace spdlog {
namespace sinks {

template<typename Mutex>
SPDLOG_INLINE ringbuffer_sink<Mutex>::ringbuffer_sink(size_t buf_size)
{
    buf_=details::circular_q<details::log_msg_buffer>(buf_size);
}

template<typename Mutex>
SPDLOG_INLINE void ringbuffer_sink<Mutex>::sink_it_(const details::log_msg &msg)
{
    buf_.push_back(details::log_msg_buffer{msg});
}

template<typename Mutex>
SPDLOG_INLINE std::vector<std::string> ringbuffer_sink<Mutex>::last(size_t lim)
{
    std::lock_guard<Mutex> lock(base_sink<Mutex>::mutex_);
    std::vector<std::string> ret;
    ret.reserve(lim);
    size_t num=0;
    for(size_t i=0; i<buf_.size(); i++){
        num++;
        memory_buf_t formatted;
        base_sink<Mutex>::formatter_->format(buf_.at(i), formatted);
        ret.push_back(fmt::to_string(formatted));
        if(lim>0 && num==lim) break;
    }
    return ret;
}
Repository: gabime/spdlog
File: include/spdlog/details/circular_q.h (C/C++)

Code before:
    }

    // Return reference to the front item.
    // If there are no elements in the container, the behavior is undefined.
    const T &front() const
    {
        return v_[head_];
    }

    T &front()
    {
        return v_[head_];
    }

    // Return number of elements actually stored
    size_t size() const
    {
        if (tail_ > head_)
        {
           return (tail_ - head_) % max_items_;
        }
        else
        {
            return max_items_ - (head_ - tail_ ) % max_items_;
        }
    }

    // Return const reference to item by index.
    // If index is out of range 0…size()-1, the behavior is undefined.
    const T &at(size_t i) const
    {
        assert(i < size());
        return v_[(head_+ i) % max_items_];
    }

    // Pop item from front.
    // If there are no elements in the container, the behavior is undefined.
    void pop_front()
    {
        head_ = (head_ + 1) % max_items_;
    }

    bool empty() const
    {
        return tail_ == head_;
    }

    bool full() const
    {
        // head is ahead of the tail by 1
        if (max_items_ > 0)

Review comment (bug):

no need to % max_items and will give wrong result if if tail_ == head_(i.e. if empty it will give max_items_)

Here is a fixed version:
```c++
size_t size() const
    {
        if (tail_ >= head_)
        {
           return (tail_ - head_);
        }
        else
        {
            return max_items_ - (head_ - tail_ );
        }
    }
```



Code after:
    }

    // Return reference to the front item.
    // If there are no elements in the container, the behavior is undefined.
    const T &front() const
    {
        return v_[head_];
    }

    T &front()
    {
        return v_[head_];
    }

    // Return number of elements actually stored
    size_t size() const
    {
        if (tail_ >= head_)
        {
           return tail_ - head_;
        }
        else
        {
            return max_items_ - (head_ - tail_ );
        }
    }

    // Return const reference to item by index.
    // If index is out of range 0…size()-1, the behavior is undefined.
    const T &at(size_t i) const
    {
        assert(i < size());
        return v_[(head_+ i) % max_items_];
    }

    // Pop item from front.
    // If there are no elements in the container, the behavior is undefined.
    void pop_front()
    {
        head_ = (head_ + 1) % max_items_;
    }

    bool empty() const
    {
        return tail_ == head_;
    }

    bool full() const
    {
        // head is ahead of the tail by 1
        if (max_items_ > 0)
Repository: gabime/spdlog
File: include/spdlog/sinks/win_eventlog_sink.h (C/C++)

Code before:
"EventMessageFile"=hex(2):25,00,73,00,79,00,73,00,74,00,65,00,6d,00,72,00,6f,\
  00,6f,00,74,00,25,00,5c,00,53,00,79,00,73,00,74,00,65,00,6d,00,33,00,32,00,\
  5c,00,6d,00,73,00,63,00,6f,00,72,00,65,00,65,00,2e,00,64,00,6c,00,6c,00,00,\
  00

-----------------------------------------------------------------------------------------*/

#pragma once

#include <winbase.h>

#include <spdlog/details/null_mutex.h>
#include <spdlog/sinks/base_sink.h>

#include <mutex>
#include <string>

namespace spdlog {
namespace sinks {

namespace win_eventlog {

namespace internal
{

struct utils
{
    /** Reports a message to stderr */
    static void report(char const* message) SPDLOG_NOEXCEPT
    {
        fprintf(stderr, "%s", message);
        fflush(stderr);
    }

    /** Reports a message to stderr */
    static void report(std::string const& message) SPDLOG_NOEXCEPT
    {
        report(message.c_str());
    }

    /** Runs a func and reports an exception to stderr if any */
    template <typename Result>
    static Result report_exceptions(std::function<Result()> func, Result defaultValue)
    {
        try
        {
            return func();
        }
        catch (std::exception const& e)
        {
            report(e.what());

Review comment (suggestion):
not needed. tooo complicated. just throw spdlog_ex with general description with thw windows error code

Code after:
"EventMessageFile"=hex(2):25,00,73,00,79,00,73,00,74,00,65,00,6d,00,72,00,6f,\
  00,6f,00,74,00,25,00,5c,00,53,00,79,00,73,00,74,00,65,00,6d,00,33,00,32,00,\
  5c,00,6d,00,73,00,63,00,6f,00,72,00,65,00,65,00,2e,00,64,00,6c,00,6c,00,00,\
  00

-----------------------------------------------------------------------------------------*/

#pragma once

#include <winbase.h>

#include <spdlog/details/null_mutex.h>
#include <spdlog/sinks/base_sink.h>

#include <mutex>
#include <string>

namespace spdlog {
namespace sinks {

namespace win_eventlog {

namespace internal
{

/** Windows error */
struct win32_error : public spdlog_ex
{
    /** Formats an error report line: "user-message: error-code (system message)" */
    static std::string format(std::string const& user_message, DWORD error_code = GetLastError())
    {
        std::string system_message;

        LPSTR format_message_result {};
        auto format_message_succeeded = FormatMessage(
            FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, nullptr,
            error_code, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), (LPSTR) &format_message_result, 0, nullptr);

        if (format_message_succeeded && format_message_result)
        {
            system_message = fmt::format(" ({})", format_message_result);
        }

        if (format_message_result)
            LocalFree((HLOCAL) format_message_result);

        return fmt::format("{}: {}{}", user_message, error_code, system_message);
    }

    win32_error(std::string const& func_name, DWORD error = GetLastError())
        : spdlog_ex(format(func_name, error))
Repository: gabime/spdlog
File: include/spdlog/tcp_sink.h (C/C++)

Code before:
public:
    tcp_sink(std::string address,int port)
    {
        if ((sock = socket(AF_INET, SOCK_STREAM, 0)) < 0)
        {
            printf("\n Socket creation error \n");
        }
        serv_addr.sin_family = AF_INET;
        serv_addr.sin_port = htons(port);
        if(inet_pton(AF_INET, address.c_str(), &serv_addr.sin_addr)<=0)
        {
            printf("\nInvalid address/ Address not supported \n");
        }
        if (connect(sock, (struct sockaddr *)&serv_addr, sizeof(serv_addr)) < 0)
        {
            printf("\nConnection Failed \n");
        }

    }
protected:
    void sink_it_(const spdlog::details::log_msg& msg) override
    {
        spdlog::memory_buf_t formatted;
        spdlog::sinks::base_sink<Mutex>::formatter_->format(msg, formatted);
        std::string message = fmt::to_string(formatted);
        send(sock , message.c_str() , message.size() , 0 );
    }

    void flush_() override
    {
    }
private:
    int sock;
    struct sockaddr_in serv_addr;
};
using tcp_sink_mt = tcp_sink<std::mutex>;
using tcp_sink_st = tcp_sink<spdlog::details::null_mutex>;


Review comment (performance):
```c++
   send(sock , formatted.data() , formatted.size() , 0 );
```
Is more efficient. No need to convert to std::string

Code after:
template<typename Mutex>
class tcp_sink : public spdlog::sinks::base_sink <Mutex>
{
public:
    tcp_sink(std::string address,int port)
    {
        if ((sock_ = socket(AF_INET, SOCK_STREAM, 0)) < 0)
        {
            SPDLOG_THROW(spdlog::spdlog_ex("Socket creation error", errno));
        }
        serv_addr_.sin_family = AF_INET;
        serv_addr_.sin_port = htons(port);
        if(inet_pton(AF_INET, address.c_str(), &serv_addr_.sin_addr)<=0)
        {
            SPDLOG_THROW(spdlog::spdlog_ex("Invalid address/ Address not supported", errno));
        }
        if (connect(sock_, (struct sockaddr *)&serv_addr_, sizeof(serv_addr_)) < 0)
        {
            SPDLOG_THROW(spdlog::spdlog_ex("Connection Failed", errno));
        }

    }
protected:
    void sink_it_(const spdlog::details::log_msg& msg) override
    {
        spdlog::memory_buf_t formatted;
        spdlog::sinks::base_sink<Mutex>::formatter_->format(msg, formatted);
        int res = send(sock_ , formatted.data() , formatted.size() , 0 );
        if(res < 0)
            SPDLOG_THROW(spdlog::spdlog_ex("Message Send Failed", errno));
    }

    void flush_() override
    {
    }
private:
    int sock_;
    struct sockaddr_in serv_addr_;
};
using tcp_sink_mt = tcp_sink<std::mutex>;
using tcp_sink_st = tcp_sink<spdlog::details::null_mutex>;