netdev - Re: [PATCH net-next V3 2/3] tools: bpftool: show filenames of pinned objects

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20171107203317.21b65c01@cakuba>
Date:   Tue, 7 Nov 2017 20:33:17 -0800
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     "Prashant Bhole" <bhole_prashant_q7@....ntt.co.jp>
Cc:     "'David S . Miller'" <davem@...emloft.net>,
        <netdev@...r.kernel.org>, "'Alexei Starovoitov'" <ast@...nel.org>,
        "'Daniel Borkmann'" <daniel@...earbox.net>,
        "'Quentin Monnet'" <quentin.monnet@...ronome.com>
Subject: Re: [PATCH net-next V3 2/3] tools: bpftool: show filenames of
 pinned objects

On Wed, 8 Nov 2017 11:34:44 +0900, Prashant Bhole wrote:
> > > +	FILE *mntfile = NULL;
> > > +	FTSENT *ftse = NULL;
> > > +	FTS *fts = NULL;
> > > +	int fd, err;
> > > +
> > > +	mntfile = setmntent("/proc/mounts", "r");
> > > +	if (!mntfile)
> > > +		return -1;
> > > +
> > > +	while ((mntent = getmntent(mntfile)) != NULL) {  
> > 
> > Please try to avoid comparisons to NULL, writing:
> > 
> > 	if (ptr)
> > 
> > is more intuitive to most C programmers than:
> > 
> > 	if (ptr != NULL)  
> 
> Jakub,
> Thank you for comments. I agree with using 'if (ptr)' for simple check, but
> here it is an assignment.
> It doesn't look intuitive and looks like a typo if I change it to:
>          while ((mntent = getmntent(mntfile))) {

I still prefer this, if you don't mind.

> >   
> > > +		char *path[] = {mntent->mnt_dir, 0};  
> > 
> > Shouldn't there be spaces after and before the curly braces?  Does  
> checkpatch --
> > strict not warn about this?  
> 
> I will add spaces. checkpatch complained about this not being static const,
> but doing so causes compiler warning because it doesn't match with signature
> of fts_open()

Right, that's OK to ignore.  Could you also make the 0 into a NULL,
though?

> I will submit v4 with changes for other comments if you are ok with replies
> above.

Thank you!