[PATCH v3] Print absolute paths in printpathn when -yy is used

[Dmitry V. Levin]

On Mon, Jun 16, 2014 at 10:29:48PM -0400, Mike Frysinger wrote:
> On Tue 17 Jun 2014 04:27:23 Dmitry V. Levin wrote:
> > On Sun, Jun 15, 2014 at 09:14:05AM +0530, zubin.mithra at gmail.com wrote:
> > > +int
> > > +print_tracee_cwd(struct tcb *tcp)
> > 
> > ... so let's make this function static bool.
> > 
> > > +{
> > > +	int link_size = sizeof("/proc/%u/cwd") + sizeof(int) * 3;
> > 
> > sizeof("/proc//cwd") + sizeof(int) * 3 is sufficient, but I do not insist.
> 
> maybe:
> 	const char fmt[] = "/proc/%u/cwd";
> 	size_t link_size = sizeof(fmt) ...
> 	...
> 	snprintf(linkpath, link_size, fmt, ...);
> 
> i'd also highlight:
>  - snprintf 2nd arg is size_t, not int

Being pedantic, I'd also make it const.

>  - return value of snprintf really needs to be checked

In general, yes, but current practice is not to check for "cannot happen"
events.

>  - assuming sizeof(int) * 3 is large enough to hold the string representation 
> of a number is really bad form.  sure, it works, but it's ugly as sin.

If it surely works and easy to read, then it is acceptable.

> all that said, this is exactly the sort of thing asprintf() was invented.  so 

I think asprintf is for arbitrary (or just too big) sizes.  In cases
like this (when buffer of 21 bytes is surely enough) I'd rather use stack.


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20140618/54dcd73a/attachment.bin>