[Asterisk-code-review] core: Stop using AST INLINE API for allocator functions. (asterisk[master])

[Richard Mudgett]

Richard Mudgett has posted comments on this change. ( https://gerrit.asterisk.org/8521 )

Change subject: core: Stop using AST_INLINE_API for allocator functions.
......................................................................


Patch Set 4: Code-Review-1

(8 comments)

https://gerrit.asterisk.org/#/c/8521/4/include/asterisk/astmm.h
File include/asterisk/astmm.h:

https://gerrit.asterisk.org/#/c/8521/4/include/asterisk/astmm.h@133
PS4, Line 133: #define calloc(a, b) \
             : 	__ast_calloc(a, b, __FILE__, __LINE__, __PRETTY_FUNCTION__)
             : #define malloc(a) \
             : 	__ast_malloc(a, __FILE__, __LINE__, __PRETTY_FUNCTION__)
             : #define free(a) \
             : 	__ast_free(a, __FILE__, __LINE__, __PRETTY_FUNCTION__)
             : #define realloc(a, b) \
             : 	__ast_realloc(a, b, __FILE__, __LINE__, __PRETTY_FUNCTION__)
             : #define strdup(a) \
             : 	__ast_strdup(a, __FILE__, __LINE__, __PRETTY_FUNCTION__)
             : #define strndup(a, b) \
             : 	__ast_strndup(a, b, __FILE__, __LINE__, __PRETTY_FUNCTION__)
             : #define asprintf(a, b, c...) \
             : 	__ast_asprintf(__FILE__, __LINE__, __PRETTY_FUNCTION__, a, b, c)
             : #define vasprintf(a, b, c) \
             : 	__ast_vasprintf(a, b, c, __FILE__, __LINE__, __PRETTY_FUNCTION__)
These should be calling the repl versions.  (Except for __ast_free().


https://gerrit.asterisk.org/#/c/8521/4/main/astmm.c
File main/astmm.c:

https://gerrit.asterisk.org/#/c/8521/4/main/astmm.c@36
PS4, Line 36: /* These functions are not used from anyplace else in Asterisk but are exposed for the
            :  * bundled pjproject.  They are not static functions so prototype declarations need to
            :  * be visible from this source. */
            : void *__ast_repl_calloc(size_t nmemb, size_t size, const char *file, int lineno, const char *func) attribute_malloc;
            : void *__ast_repl_malloc(size_t size, const char *file, int lineno, const char *func) attribute_malloc;
            : void *__ast_repl_realloc(void *ptr, size_t size, const char *file, int lineno, const char *func);
            : char *__ast_repl_strdup(const char *s, const char *file, int lineno, const char *func) attribute_malloc;
            : char *__ast_repl_strndup(const char *s, size_t n, const char *file, int lineno, const char *func) attribute_malloc;
            : int __ast_repl_asprintf(const char *file, int lineno, const char *func, char **strp, const char *format, ...)
            : 	__attribute__((format(printf, 5, 6)));
            : int __ast_repl_vasprintf(char **strp, const char *format, va_list ap, const char *file, int lineno, const char *func)
            : 	__attribute__((format(printf, 2, 0)));
These need to be moved to astmm.h so redirected stdlib versions go to the MALLOC_DEBUG replacements.


https://gerrit.asterisk.org/#/c/8521/4/main/astmm.c@522
PS4, Line 522: 	DEBUG_CHAOS_RETURN(DEBUG_CHAOS_ALLOC_CHANCE, NULL);
This is in __ast_alloc_region().


https://gerrit.asterisk.org/#/c/8521/4/main/astmm.c@1572
PS4, Line 1572: void *__ast_calloc_cache(size_t nmemb, size_t size, const char *file, int lineno, const char *func)
We need to keep __ast_repl_calloc_cache().  However, we can make the two __ast_repl_calloc_cache() versions static functions.  As currently coded, DEBUG_CHAOS returning will not get the MALLOC_FAILURE_MSG if it decides to randomly fail the allocation.

We need the __ast_calloc_cache() to do just like __ast_calloc() but call __ast_repl_calloc_cache() instead.


https://gerrit.asterisk.org/#/c/8521/4/main/astmm.c@1629
PS4, Line 1629: 	if (res < 0) {
              : 		/*
              : 		 * *strp is undefined so set to NULL to ensure it is
              : 		 * initialized to something useful.
              : 		 */
              : 		*strp = NULL;
              : 	}
This should be removed as it is not done by the stdlib.


https://gerrit.asterisk.org/#/c/8521/4/main/astmm.c@1648
PS4, Line 1648: 	if (res < 0) {
              : 		/*
              : 		 * *strp is undefined so set to NULL to ensure it is
              : 		 * initialized to something useful.
              : 		 */
              : 		*strp = NULL;
              : 	}
This should go in the __ast_vasprintf as it is not done by the stdlib.


https://gerrit.asterisk.org/#/c/8521/4/third-party/pjproject/patches/asterisk_malloc_debug.h
File third-party/pjproject/patches/asterisk_malloc_debug.h:

https://gerrit.asterisk.org/#/c/8521/4/third-party/pjproject/patches/asterisk_malloc_debug.h@28
PS4, Line 28: int __ast_asprintf(const char *file, int lineno, const char *func, char **strp, const char *format, ...)
This file needs to be reverted and then the attribute_malloc added where appropriate.


https://gerrit.asterisk.org/#/c/8521/4/third-party/pjproject/patches/asterisk_malloc_debug.c
File third-party/pjproject/patches/asterisk_malloc_debug.c:

https://gerrit.asterisk.org/#/c/8521/4/third-party/pjproject/patches/asterisk_malloc_debug.c@25
PS4, Line 25: int __ast_asprintf(const char *file, int lineno, const char *func, char **strp, const char *format, ...)
This file needs to be reverted.



-- 
To view, visit https://gerrit.asterisk.org/8521
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9df4377f74bdbb627461b27a473123e05525887
Gerrit-Change-Number: 8521
Gerrit-PatchSet: 4
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Thu, 15 Mar 2018 00:51:31 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180314/34840e2e/attachment-0001.html>