Re: [PATCH 3/3] fwd, conf: Probe host's ephemeral ports

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2323 bytes --]

On Thu, Aug 29, 2024 at 04:59:55AM +0200, Stefano Brivio wrote:
> On Thu, 29 Aug 2024 11:29:30 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Wed, Aug 28, 2024 at 12:22:18PM +0200, Laurent Vivier wrote:
> > > On 28/08/2024 07:56, David Gibson wrote:  
> > > >
> > > > [...]
> > > >
> > > > +	lineread_init(&lr, fd);
> > > > +	len = lineread_get(&lr, &line);
> > > > +	if (len < 0)
> > > > +		goto parse_err;
> > > > +
> > > > +	tab = strchr(line, '\t');
> > > > +	if (!tab)
> > > > +		goto parse_err;
> > > > +	*tab = '\0';
> > > > +
> > > > +	errno = 0;
> > > > +	min = strtol(line, &end, 10);
> > > > +	if (*end || errno)
> > > > +		goto parse_err;
> > > > +
> > > > +	errno = 0;
> > > > +	max = strtol(tab + 1, &end, 10);
> > > > +	if (*end || errno)
> > > > +		goto parse_err;  
> > > 
> > > As /proc files are well formated, why don't you use fscanf()?
> > > Something like:
> > > 
> > >         FILE *f;
> > > 
> > >         f = fopen(PORT_RANGE_SYSCTL, "r");
> > > 	if (f == NULL) {
> > > 		warn("Unable to parse %s", PORT_RANGE_SYSCTL);
> > > 		return;
> > > 	}
> > >         ret = fscanf(f, "%d %d", &min, &max);
> > >         fclose(f);
> > >         if (ret != 2)
> > >                 goto parse_error;  
> > 
> > Hm, maybe.  I never feel like I know exactly what the parse rules for
> > scanf() are, so I tend to avoid it.  Stefano, any thoughts?
> 
> The issue with fopen() and ffscanf() is that they need dynamic
> allocation, see also 32d07f5e59f2 ("passt, pasta: Completely avoid
> dynamic memory allocation").

Ah, good point, I'd forgotten that.

> Still, here, we could use lineread_get() and sscanf() as we do in fwd.c,
> procfs_scan_listen(), but I don't really have a preference. If you find
> that scanf() is confusing for you, perhaps it would be better to use
> something simpler as you did. On the other hand it would be 2 lines of
> code instead of (somewhat bug-prone) 14 lines.

True.  Ok, I've switched over to sscanf() for the next spin.

> I'm not sure, I'd say whatever you feel most comfortable with...
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]