shithub: openh264

Info  •  Files  •  Log  •  Branches

Don't manually null terminate after calling SNPRINTF

The following pattern is unsafe on all platforms:
 n = SNPRINTF(buf, ...);
 buf[n] = '\0';

On windows, the _snprintf variants return a negative number
if the buffer was too small, thus buf[n] would be outside
of (before the start of) the buffer.

On other platforms, the C99 snprintf function returns the
total number of characters which would have been written if
the buffer had been large enough, which can be larger than
the buffer size itself, and thus buf[n] would be beyond the
end of the buffer.

The C99 snprintf function always null terminate the buffer.
These invocations of SNPRINTF are within !WIN32, so we can
be sure that the SNPRINTF call itself already null terminated
the buffer.

--- a/codec/encoder/core/src/slice_multi_threading.cpp

+++ b/codec/encoder/core/src/slice_multi_threading.cpp

@@ -443,7 +443,6 @@

   while (iIdx < iThreadNum) {

 #if defined(__GNUC__) && !defined(_WIN32)	// for posix threading

     str_t name[SEM_NAME_MAX] = {0};

-    int32_t used_len = 0;

     WELS_THREAD_ERROR_CODE err = 0;

 #endif//__GNUC__

     pSmt->pThreadPEncCtx[iIdx].pWelsPEncCtx	= (void*) (*ppCtx);

@@ -462,8 +461,7 @@

 #if defined(ENABLE_TRACE_MT)

     WelsLog ((*ppCtx), WELS_LOG_INFO, "[MT] Open pUpdateMbListEvent%d named(%s) ret%d err%d\n", iIdx, name, err, errno);

 #endif

-    used_len = SNPRINTF (name, SEM_NAME_MAX, "fu%d%p", iIdx, (void*) (*ppCtx));

-    name[used_len] = '\0';

+    SNPRINTF (name, SEM_NAME_MAX, "fu%d%p", iIdx, (void*) (*ppCtx));

     err = WelsEventOpen (&pSmt->pFinUpdateMbListEvent[iIdx], name);

 #if defined(ENABLE_TRACE_MT)

     WelsLog ((*ppCtx), WELS_LOG_INFO, "[MT] Open pFinUpdateMbListEvent%d named(%s) ret%d err%d\n", iIdx, name, err, errno);

@@ -477,14 +475,12 @@

     WelsEventInit (&pSmt->pFinSliceCodingEvent[iIdx]);

     WelsEventInit (&pSmt->pExitEncodeEvent[iIdx]);

 #else

-    used_len = SNPRINTF (name, SEM_NAME_MAX, "sc%d%p", iIdx, (void*) (*ppCtx));

-    name[used_len] = '\0';

+    SNPRINTF (name, SEM_NAME_MAX, "sc%d%p", iIdx, (void*) (*ppCtx));

     err = WelsEventOpen (&pSmt->pSliceCodedEvent[iIdx], name);

 #if defined(ENABLE_TRACE_MT)

     WelsLog ((*ppCtx), WELS_LOG_INFO, "[MT] Open pSliceCodedEvent%d named(%s) ret%d err%d\n", iIdx, name, err, errno);

 #endif

-    used_len = SNPRINTF (name, SEM_NAME_MAX, "rc%d%p", iIdx, (void*) (*ppCtx));

-    name[used_len] = '\0';

+    SNPRINTF (name, SEM_NAME_MAX, "rc%d%p", iIdx, (void*) (*ppCtx));

     err = WelsEventOpen (&pSmt->pReadySliceCodingEvent[iIdx], name);

 #if defined(ENABLE_TRACE_MT)

     WelsLog ((*ppCtx), WELS_LOG_INFO, "[MT] Open pReadySliceCodingEvent%d = 0x%p named(%s) ret%d err%d\n", iIdx,

@@ -577,19 +573,15 @@

 #endif//DYNAMIC_SLICE_ASSIGN && TRY_SLICING_BALANCE

 #else

     str_t ename[SEM_NAME_MAX] = {0};

-    int32_t used_len = 0;

     // length of semaphore name should be system constrained at least on mac 10.7

     SNPRINTF (ename, SEM_NAME_MAX, "sc%d%p", iIdx, (void*) (*ppCtx));

     WelsEventClose (pSmt->pSliceCodedEvent[iIdx], ename);

-    used_len = SNPRINTF (ename, SEM_NAME_MAX, "rc%d%p", iIdx, (void*) (*ppCtx));

-    ename[used_len] = '\0';

+    SNPRINTF (ename, SEM_NAME_MAX, "rc%d%p", iIdx, (void*) (*ppCtx));

     WelsEventClose (pSmt->pReadySliceCodingEvent[iIdx], ename);

 #if defined(DYNAMIC_SLICE_ASSIGN) && defined(TRY_SLICING_BALANCE)

-    used_len = SNPRINTF (ename, SEM_NAME_MAX, "ud%d%p", iIdx, (void*) (*ppCtx));

-    ename[used_len] = '\0';

+    SNPRINTF (ename, SEM_NAME_MAX, "ud%d%p", iIdx, (void*) (*ppCtx));

     WelsEventClose (pSmt->pUpdateMbListEvent[iIdx], ename);

-    used_len = SNPRINTF (ename, SEM_NAME_MAX, "fu%d%p", iIdx, (void*) (*ppCtx));

-    ename[used_len] = '\0';

+    SNPRINTF (ename, SEM_NAME_MAX, "fu%d%p", iIdx, (void*) (*ppCtx));

     WelsEventClose (pSmt->pFinUpdateMbListEvent[iIdx], ename);

 #endif//DYNAMIC_SLICE_ASSIGN && TRY_SLICING_BALANCE

 #endif//_WIN32