netdev - Re: [RFC PATCH 2/2] PID-based packet filtering support

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALKxAXUw1HU8Dqz8+DiCe_p-j3j0tV5=89sFaqLRLiQZA3BjUA@mail.gmail.com>
Date:	Thu, 5 Apr 2012 11:30:20 +0100
From:	Nuno Martins <nuno.martins@...xamagica.pt>
To:	chetan loke <loke.chetan@...il.com>
Cc:	netdev <netdev@...r.kernel.org>,
	Alfredo Matos <alfredo.matos@...xamagica.pt>,
	Paulo Trezentos <paulo.trezentos@...xamagica.pt>
Subject: Re: [RFC PATCH 2/2] PID-based packet filtering support

On 4 April 2012 19:57, chetan loke <loke.chetan@...il.com> wrote:
> On Wed, Apr 4, 2012 at 5:16 AM, Nuno Martins
> <nuno.martins@...xamagica.pt> wrote:
>
>
>> +int get_local_packet_info_from_fd(unsigned int fd, struct packetInfo *ret)
>
> rename 'ret' to 'pi' or pkt_info or pick a name (must match the
> struct). And not just here but everywhere else.
>

Corrected.

>> +{
>> +       struct file *f = fget(fd);
>> +
>> +       if (f != NULL) {
>> +               fput(f);
>
> why not 'fput' after you return from 'get_local_packet_info_from_file' ?
>
>> +               return get_local_packet_info_from_file(f, ret);
>> +       }
>> +       return -3;
>> +}
>> +

Yes, it make more sense to make the fput after I have done with the file.


>
>
>> +int get_local_packet_info_from_file(struct file *f, struct packetInfo *ret)
>> +{
>> +       struct socket *socket = NULL;
>> +       short type;
>> +       unsigned short family;
>> +       int err = 0;
>> +
>> +       if (f != NULL) {
>
> This check is redundant because the
> caller(get_local_packet_info_from_fd) checks for NULL.

It's now corrected, it was there because in another function in
task_monitor in the beginning I was not checking for NULL.
So now the check for NULL in get_local_packet_info_from_file
is redundant, is going to be taken away.

>
> Move this function before get_local_packet_info_from_fd().
>
> re-arranged code looks like:
>
> int get_local_packet_info_from_file(...) {
> ....
> }
>
> int get_local_packet_info_from_fd(...) {
>   ...
>   ... get_local_packet_info_from_file(f, ret);
>   ....
> }
>
>

Re-factored the code to look like that.

Thanks for you feedback, and all will be corrected on the next version and on
the repository.

> Chetan

Nuno Martins
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html