Re: [LTP] [PATCH v4 1/2] Add SAFE_MPROTECT() macro

[Petr Vorel <pvorel@suse.cz>]

Hi,

> Hi!
> > +	int rval;
> > +	char prot_buf[16];
> > +
> > +	switch (prot) {
> > +	case PROT_NONE:
> > +		snprintf(prot_buf, 16, "PROT_NONE");
> > +		break;
> > +	case PROT_WRITE:
> > +		snprintf(prot_buf, 16, "PROT_WRITE");
> > +		break;
> > +	case PROT_READ:
> > +		snprintf(prot_buf, 16, "PROT_READ");
> > +		break;
> > +	case PROT_EXEC:
> > +		snprintf(prot_buf, 16, "PROT_EXEC");
> > +		break;
> > +	default:
> > +		snprintf(prot_buf, 16, "UNKNOWN");
> > +		break;
> > +	}

> This is ugly and does not work.

> First of all we can just do:

> 	char *prot_name;


> 	switch (prot) {
> 	case PROT_NONE:
> 		prot_name = "PROT_NONE";
> 	break;
> 	...
> 	}

> And secondly it does not work for common combinations, like (PROT_READ | PROT_WRITE).

> So I guess that the easiest solution is to walk the bits and append the
> string something as:

> #define PROT_READ_NAME "PROT_READ | "
> #define PROT_WRITE_NAME "PROT_WRITE | "
> #define PROT_EXEC_NAME "PROT_EXEC | "

nit: maybe using stringification?

#define FLAG(f) f, #f " | "

FLAG(PROT_READ)

> static const char *prot_to_str(int prot, char *buf, size_t buf_len)
> {
> 	char *orig_buf = buf;

> 	if (buf_len < sizeof(PROT_READ_NAME) + sizeof(PROT_WRITE_NAME) + sizeof(PROT_EXEC_NAME))
> 		return "BUFFER TOO SMALL!!!";

> 	if (prot == 0)
> 		return "PROT_NONE";

> 	*buf = 0;

> 	if (prot & PROT_READ) {
> 		strcpy(buf, PROT_READ_NAME);
> 		buf += sizeof(PROT_READ_NAME)-1;
> 	}

> 	if (prot & PROT_WRITE) {
> 		strcpy(buf, PROT_WRITE_NAME);
> 		buf += sizeof(PROT_WRITE_NAME)-1;
> 	}

> 	if (prot & PROT_EXEC) {
> 		strcpy(buf, PROT_EXEC_NAME);
> 		buf += sizeof(PROT_EXEC_NAME)-1;
> 	}

> 	if (orig_buf != buf)
> 		buf[-3] = 0;

> 	return buf;
> }

> Also I would put the code that translates the prot into string into a
> separate function because that code should be used in the safe_mmap() as
> well.

+1

> > +	tst_res_(file, lineno, TDEBUG,
> > +		"mprotect(%p, %d, %s)", addr, len, prot_buf);

> Can we print hexadecimal value of the prot here as well?

+1

> 		"mprotect(%p, %d, %s(%x))", addr, len, prot_to_str(prot, buf, sizeof(buf)), prot);

Kind regards,
Petr

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