Re: [LTP] [PATCH v5 1/3] Add SAFE_MPROTECT() macro

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> +static void prot_to_str(const int prot, char *buf)
> +{
> +	char *ptr = buf;
>

I would still put an explicit check for the buffer size here, so that we
are sure that the complete combination of READ|WRITE|EXEC would fit, but
I guess that it's fine as long as this is an internal static function
and not part of the API.

> +	if (prot == PROT_NONE) {
> +		strcpy(buf, "PROT_NONE");
> +		return;
> +	}
> +
> +	if (prot & PROT_READ) {
> +		strcpy(ptr, PROT_FLAG_STR(PROT_READ));
> +		ptr += sizeof(PROT_FLAG_STR(PROT_READ)) - 1;
> +	}
> +
> +	if (prot & PROT_WRITE) {
> +		strcpy(ptr, PROT_FLAG_STR(PROT_WRITE));
> +		ptr += sizeof(PROT_FLAG_STR(PROT_WRITE)) - 1;
> +	}
> +
> +	if (prot & PROT_EXEC) {
> +		strcpy(ptr, PROT_FLAG_STR(PROT_EXEC));
> +		ptr += sizeof(PROT_FLAG_STR(PROT_EXEC)) - 1;
> +	}
> +
> +	if (buf != ptr)
> +		ptr[-3] = 0;
> +}
> +
>  static inline void *safe_mmap(const char *file, const int lineno,
>                                void *addr, size_t length,
>                                int prot, int flags, int fd, off_t offset)
> @@ -287,6 +318,35 @@ static inline void *safe_mmap(const char *file, const int lineno,
>  	safe_mmap(__FILE__, __LINE__, (addr), (length), (prot), \
>  	(flags), (fd), (offset))
>  
> +static inline int safe_mprotect(const char *file, const int lineno,
> +	char *addr, size_t len, int prot)
> +{
> +	int rval;
> +	char *prot_buf;
> +
> +	prot_buf = (char*) safe_malloc(file, lineno, 0, 512);

Why are we allocating the buffer? Why not just prot_buf[512] ?

Also the cast to (char*) is never needed in C as void* is automatically
converted to any type of a pointer without explicit cast.


Otherwise it looks good. You can add my Reviewed-by: if you change the
malloc to an array on the stack.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp