[Pdbg] [PATCH 05/10] main: Add an option for path based targetting

[Alistair Popple]

On Wednesday, 31 October 2018 3:01:19 PM AEDT Alistair Popple wrote:
> Hi Amitay,
> 
> On Tuesday, 2 October 2018 4:04:26 PM AEDT Amitay Isaacs wrote:
> > +static bool pathsel_add(char *format, ...) __attribute__((format (printf,
> > 1, 2))); +
> > +static bool pathsel_add(char *format, ...)
> > +{
> > +	va_list ap;
> > +	char path[1024];
> > +	int len;
> > +
> > +	va_start(ap, format);
> > +
> > +	len = vsnprintf(path, sizeof(path), format, ap);
> 
> I can't quite figure out what's going on here. As far as I can tell you're
> just passing a single string (optarg) to this function and adding it to the
> pathsel array. Why do we need the vsnprintf and the va_args? Am I missing
> something?

Oh, I missed it's use in the subsequent patch. However what happens if the 
path on the command line contains a "%"? I get the below error:

$ ./pdbg -P "fsi0/pib%d" getscom 0xf000f
Invalid index '80'

So it seems like the vnsprintf is going to interpret the path as a format 
string which I'd like to see fixed. Thanks!

- Alistair
 
> Thanks.
> 
> > +	if (len > sizeof(path)) {
> > +		va_end(ap);
> > +		return false;
> > +	}
> > +
> > +	va_end(ap);
> > +
> > +	if (pathsel_count == MAX_PATH_ARGS) {
> > +		fprintf(stderr, "Too many path arguments\n");
> > +		return false;
> > +	}
> > +
> > +	pathsel[pathsel_count] = strdup(path);
> > +	assert(pathsel[pathsel_count]);
> > +	pathsel_count++;
> > +
> > +	return true;
> > +}
> > +
> > 
> >  static bool parse_options(int argc, char *argv[])
> >  {
> >  
> >  	int c;
> > 
> > @@ -263,6 +300,7 @@ static bool parse_options(int argc, char *argv[])
> > 
> >  		{"cpu",			required_argument,	NULL,	'l'},
> >  
> >  #endif
> >  
> >  		{"debug",		required_argument,	NULL,	'D'},
> > 
> > +		{"path",		required_argument,	NULL,	'P'},
> > 
> >  		{"shutup",		no_argument,		NULL,	'S'},
> >  		{"version",		no_argument,		NULL,	'V'},
> >  		{NULL,			0,			NULL,     0}
> > 
> > @@ -275,7 +313,7 @@ static bool parse_options(int argc, char *argv[])
> > 
> >  	memset(l_list, 0, sizeof(l_list));
> >  	
> >  	do {
> > 
> > -		c = getopt_long(argc, argv, "+ab:c:d:hp:s:t:D:SV" PPC_OPTS,
> > +		c = getopt_long(argc, argv, "+ab:c:d:hp:s:t:D:P:SV" PPC_OPTS,
> > 
> >  				long_opts, NULL);
> >  		
> >  		if (c == -1)
> >  		
> >  			break;
> > 
> > @@ -361,6 +399,11 @@ static bool parse_options(int argc, char *argv[])
> > 
> >  				fprintf(stderr, "Invalid slave address '%s'\n", optarg);
> >  			
> >  			break;
> > 
> > +		case 'P':
> > +			if (!pathsel_add(optarg))
> > +				opt_error = true;
> > +			break;
> > +
> > 
> >  		case 'S':
> >  			progress_shutup();
> >  			break;
> > 
> > @@ -647,6 +690,11 @@ static bool target_selection(void)
> > 
> >  			target_unselect(fsi);
> >  	
> >  	}
> > 
> > +	if (pathsel_count) {
> > +		if (!path_target_parse(pathsel, pathsel_count))
> > +			return false;
> > +	}
> > +
> > 
> >  	return true;
> >  
> >  }