UniquePtr: Support SrsUniquePtr to replace SrsAutoFree. v6.0.136 by winlinvip · Pull Request #4109 · ossrs/srs

winlinvip · 2024-07-08T08:54:39Z

To manage an object:

// Before
MyClass* ptr = new MyClass();
SrsAutoFree(MyClass, ptr);
ptr->do_something();

// Now
SrsUniquePtr<MyClass> ptr(new MyClass());
ptr->do_something();

To manage an array of objects:

// Before
char* ptr = new char[10];
SrsAutoFreeA(char, ptr);
ptr[0] = 0xf;

// Now
SrsUniquePtr<char[]> ptr(new char[10]);
ptr[0] = 0xf;

In fact, SrsUniquePtr is a limited subset of SrsAutoFree, mainly managing pointers and arrays. SrsUniquePtr is better than SrsAutoFree because it has the same API to standard unique ptr.

SrsUniquePtr<MyClass> ptr(new MyClass());
ptr->do_something();
MyClass* p = ptr.get();

SrsAutoFree actually uses a pointer to a pointer, so it can be set to NULL, allowing the pointer's value to be changed later (this usage is different from SrsUniquePtr).

// OK to free ptr correctly.
MyClass* ptr;
SrsAutoFree(MyClass, ptr);
ptr = new MyClass();

// Crash because ptr is an invalid pointer.
MyClass* ptr;
SrsUniquePtr<MyClass> ptr(ptr);
ptr = new MyClass();

Additionally, SrsAutoFreeH can use specific release functions, which SrsUniquePtr does not support.

Co-authored-by: Jacob Su suzp1984@gmail.com

trunk/src/utest/srs_utest_protocol.cpp

suzp1984 · 2024-07-08T10:14:17Z

trunk/src/core/srs_core_deprecated.hpp

+#ifndef SRS_CORE_DEPRECATED_HPP
+#define SRS_CORE_DEPRECATED_HPP
+
+#include <srs_core.hpp>


this include header,srs_core.hpp, is useless, can be removed.

I want to make sure the first included file is this one.

Won't fix.

suzp1984 · 2024-07-08T10:24:47Z

trunk/src/core/srs_core_deprecated.cpp

+// SPDX-License-Identifier: MIT
+//
+
+#include <srs_core_deprecated.hpp>


I don't know whether it's a code conventions or not, but an empty cpp, seems useless.

It will cause the utest fail. Please file another PR if you want to remove it.

Won't fix.

suzp1984 · 2024-07-08T12:15:05Z

trunk/src/app/srs_app_http_conn.cpp

    // process all http messages.
-    err = process_requests(&last_req);
+    SrsRequest* last_req_raw = NULL;
+    err = process_requests(&last_req_raw);


err need to freed and processed if it's not srs_success, otherwise err != srs_success would be a memory leak.

The err is returned:

err = process_requests(&last_req_raw); if ((r0 = on_disconnect(last_req.get())) != srs_success) { err = srs_error_wrap(err, "on disconnect %s", srs_error_desc(r0).c_str()); srs_freep(r0); } return err;

Won't fix.

trunk/src/app/srs_app_recv_thread.cpp

suzp1984 · 2024-07-08T14:46:49Z

trunk/src/app/srs_app_rtc_conn.cpp

    }

-    srs_assert(consumer);
+    srs_assert(consumer_raw);


consumer_raw will alway has value, no need to check with assert?

srs/trunk/src/app/srs_app_rtc_source.cpp

Lines 550 to 562 in baf22d0

srs_error_t SrsRtcSource::create_consumer(SrsRtcConsumer*& consumer)

{

srs_error_t err = srs_success;

consumer = new SrsRtcConsumer(this);

consumers.push_back(consumer);

stream_die_at_ = 0;

// TODO: FIXME: Implements edge cluster.

return err;

}

Not a bug or logic error.

Won't fix.

suzp1984 · 2024-07-08T15:17:05Z

trunk/src/app/srs_app_rtc_source.cpp

        // We must free it, should never use RTP packets to free it,
        // because more than one RTP packet will refer to it.
-        SrsAutoFree(SrsRtpRawNALUs, raw);
+        SrsUniquePtr<SrsRtpRawNALUs> raw(raw_raw);


I would rather use a srs_freep(raw_raw) manually in this else scope to simple the code.

Nop, it's used later:

SrsUniquePtr<SrsRtpRawNALUs> raw(raw_raw); uint8_t header = raw->skip_first_byte();

Won't fix.

trunk/src/app/srs_app_rtmp_conn.cpp

suzp1984 · 2024-07-08T15:41:13Z

trunk/src/app/srs_app_srt_conn.cpp

    }
-    srs_assert(consumer);
+
+    srs_assert(consumer_raw);


consumer_raw always true here

Not a bug or an error.

Won't fix.

suzp1984

all done

trunk/scripts/copy_to_gits.sh

UniquePtr: Support SrsUniquePtr to replace SrsAutoFree.

0c84717

winlinvip added the EnglishNative This issue is conveyed exclusively in English. label Jul 8, 2024

winlinvip marked this pull request as ready for review July 8, 2024 08:55