added 8 commits

• 5625f6f2...cab8791f - 7 commits from branch GNOME:master
• b2e590a4 - Fix merge conflict in jhbuild_modif branch

Compare with previous version

changed the description

mentioned in merge request gnome-usage!34

added 78 commits

• 8bd537f6 - Add directory traversal of /proc
• a9cad964 - Add file parsing for /proc/net/tcp
• 193190f9 - Add parsing of network connections from /proc/net/ files
• 27add439 - Add test for netsocket.c
• 0482134a - Add hashTable to store inode and pid
• 53394f32 - Add test for proc_inode_parse.c
• d6963d4f - Add glib version of dir traversal
• 295119c3 - dev_handles : Open pcap handles for network interfaces
• 72f85edc - Test for checking the opened pcap handles for network devices
• f8a584ec - Change the dir name approach to GFile
• 053936c4 - GFile changes in the args
• 0c311572 - Add GError for error handling
• 016c3746 - Switched to GINT_TO_POINTER
• 670736c1 - Replace malloc with g_slice_new
• fffd2464 - Minor changes in test file
• aba855b2 - Fix warning
• 5be88740 - coding style fixes
• 1e57f5ba - Add Interface's local address
• 99959fa3 - Add local address to interfaces
• ba33364f - Add functions for manipulating packets
• 6a85c65f - Add connections details to keep track of relevant packets
• 4a5b9bc2 - Add process details to track corresponding connections
• 0b5560fc - Add callback for tcp
• ea04658b - Add more packet functions
• 5b95d02b - Add hash table for socket details and inode
• 9aa44716 - Add packet processing functions
• 515b4a59 - Modifications in packet function prototypes
• a2a6f67c - Fix struct member name issues
• 24c67dd9 - Add structs for network stats output
• 5b1cfa22 - Add main testing file
• c3e87809 - Redefinition issues fixed
• d90c5516 - Add packet hash to packet struct
• bda2f65b - dev_handles.c : Modify packet processing functions
• f739c742 - dev_handles.h: Add definition of packet headers
• 215e0874 - connection: Modify the connection matching functions
• 55478d8a - This commit has to be broken down into patches
• bfecdbbe - Separate the headers and tests
• 99937f69 - add missing files to Makefile
• 5d66652a - Add Gio dependency
• 97188a52 - Add Symbols to MakeFile
• d47b8dfa - Add LIBGTOP prefix to header guards
• 632e5de0 - Change Process_list to GSList
• 399bc42e - Change Conn_list to GSList
• a0d8a145 - Replace u_int data types by their GLib alternatives
• 56932f95 - Change Packet_list to GSList
• 01b8aa02 - Add new headers to the list of introspectables
• 8b234c41 - Change the network_stats_entry struct
• 015a0850 - Add network stats cmd to the daemon
• 01a9a738 - Change flags to enable Daemon for Linux
• 9eb6cf9b - Define the libgtop_sysdeps_suid library for Linux
• 7d763cd1 - Add glibtop_init_p in Linux
• 196a1482 - Change the local_addr field in struct glibtop_socket
• 33338076 - Add glibtop_open_p function
• f9378661 - Remove the error logs
• c34bf34c - Add DBus Interface to get the stats
• be0bc1c6 - Change the output GVariant
• 47c2da9d - Add capture status function
• d169d3df - Add capture initializer function to DBus
• 0b892b00 - Change Daemon settings for Linux
• 518fd81b - Move the DBus example to examples dir
• 4d72a46c - Add packet capture init function to stats
• fcbfca91 - Add GPtrArray for the dbus_stats
• 7e017fb2 - Add parameter in the get_stats_instance invocation
• c401e91e - Change the condition for keeping the capture active
• f7a9f290 - Rename the toggle_capture_status function
• 91b27483 - WIP: Add the dictionary for DBus Messages
• 05e01a13 - Remove proc_name from various structs
• bdcf3abc - Fix the issue in dictionary in GVariants
• 07bb0648 - Add g_timeout_add in the dbus interface
• 2cdddea6 - Fix timeout issue in DBus interface
• 738086d0 - Add check to stop the packet capture
• 38cee26a - Fix segfault in GetStats
• 502a95a0 - Add config files for DBus
• d6d5e201 - Add set and reset functions to dbus-interface
• f1f229c8 - Discard values ~0.00 in the network stats
• e9bdd8ac - Activate the packet capture in the network test example
• bc3a0250 - Use g_error wherever required
• 050ba450 - Fix warnings during compilation

Compare with previous version

I feel that pcap is missing here from the list of modules to check for, the PCAP_REQUIRED variable is not used.

I added this in the recent commit ,is this fine ?

also for pcap we need to use pcap-config as opposed to pkg-config

Ok, I missed the PCAP_CONFIG part. It's fine this way, marking as resolved.

I'm sure these conflict markers do not belong here.

changed this line in version 4 of the diff

I'm sure these conflict markers do not belong here.

changed this line in version 4 of the diff

I'm sure these conflict markers do not belong here.

changed this line in version 4 of the diff

This harcoded path is surely not correct, but no idea yet what to use here, we'll probably need to generate this dbus service file using autotools to include the real path based on the prefix autotools was configured with.

We will surely need the dbus_interface installed, so let's move it to somewhere else. Probably not inside examples but rather inside the daemon directory.

We shouldn't have such conversion functions in libgtop, there is g_format_size in glib.

changed this line in version 6 of the diff

to_kb is defined as static in the net_process.c file. So it shouldn't be in the h file at all.

I did remove this just sometime back before you commented :)

@robertroth I've changed the API to just return number of bytes over dbus.

added 78 commits

• 6d36f952 - Add directory traversal of /proc
• a99fcf92 - Add file parsing for /proc/net/tcp
• ed1bdf8f - Add parsing of network connections from /proc/net/ files
• 8946ce74 - Add test for netsocket.c
• 291d555d - Add hashTable to store inode and pid
• 517f750e - Add test for proc_inode_parse.c
• 74e56b58 - Add glib version of dir traversal
• 4f4127fd - dev_handles : Open pcap handles for network interfaces
• 0df6c8df - Test for checking the opened pcap handles for network devices
• b4be0c8b - Change the dir name approach to GFile
• 7fd26234 - GFile changes in the args
• b58fdef5 - Add GError for error handling
• 11cacfe4 - Switched to GINT_TO_POINTER
• ca8515dd - Replace malloc with g_slice_new
• 1c1a8515 - Minor changes in test file
• 6519029f - Fix warning
• ef8e091e - coding style fixes
• 830a9430 - Add Interface's local address
• 297d7ad4 - Add local address to interfaces
• a6ae2722 - Add functions for manipulating packets
• 001ea6c5 - Add connections details to keep track of relevant packets
• 5c883514 - Add process details to track corresponding connections
• 64ade368 - Add callback for tcp
• 0173a0cf - Add more packet functions
• d2460b74 - Add hash table for socket details and inode
• 1a0608ad - Add packet processing functions
• 5fd47603 - Modifications in packet function prototypes
• c6ba6983 - Fix struct member name issues
• 1faede8c - Add structs for network stats output
• 447e579e - Add main testing file
• 2df84811 - Redefinition issues fixed
• 818d2e6a - Add packet hash to packet struct
• b4cde82d - dev_handles.c : Modify packet processing functions
• c9475686 - dev_handles.h: Add definition of packet headers
• 56aebb48 - connection: Modify the connection matching functions
• d91445da - This commit has to be broken down into patches
• 9c0f7b74 - Separate the headers and tests
• 87ee8482 - add missing files to Makefile
• 1c32220e - Add Gio dependency
• 63e285a3 - Add Symbols to MakeFile
• b94e173b - Add LIBGTOP prefix to header guards
• 59ae3667 - Change Process_list to GSList
• cc7d0ed9 - Change Conn_list to GSList
• d5e5a101 - Replace u_int data types by their GLib alternatives
• 4d4dad26 - Change Packet_list to GSList
• a09b0a54 - Add new headers to the list of introspectables
• 7f9f3d1e - Change the network_stats_entry struct
• 99e3efa0 - Add network stats cmd to the daemon
• 669d40be - Change flags to enable Daemon for Linux
• 03fdf8d6 - Define the libgtop_sysdeps_suid library for Linux
• fb595da7 - Add glibtop_init_p in Linux
• e980c6bd - Change the local_addr field in struct glibtop_socket
• 735333be - Add glibtop_open_p function
• 845517cf - Remove the error logs
• 343e582b - Add DBus Interface to get the stats
• 990aadee - Change the output GVariant
• c046d6a3 - Add capture status function
• 5053b599 - Add capture initializer function to DBus
• 21e6383f - Change Daemon settings for Linux
• 23d50dd4 - Move the DBus example to examples dir
• d8e85a37 - Add packet capture init function to stats
• ddbfb8fc - Add GPtrArray for the dbus_stats
• a6272a85 - Add parameter in the get_stats_instance invocation
• 33907c68 - Change the condition for keeping the capture active
• a502cfea - Rename the toggle_capture_status function
• f7ec73dc - WIP: Add the dictionary for DBus Messages
• 18fdd98a - Remove proc_name from various structs
• 7a01cd6a - Fix the issue in dictionary in GVariants
• d46b5aa0 - Add g_timeout_add in the dbus interface
• bcfde989 - Fix timeout issue in DBus interface
• 216086de - Add check to stop the packet capture
• 68d622ba - Fix segfault in GetStats
• 5fd8826b - Add config files for DBus
• a73cb0e0 - Add set and reset functions to dbus-interface
• cb31e29c - Discard values ~0.00 in the network stats
• 8865f9c7 - Activate the packet capture in the network test example
• aa2f29c0 - Use g_error wherever required
• cad22c04 - Fix warnings during compilation

Compare with previous version

Let's leave this convertsion to the GUIs.

Now I've changed the stats to be returned as kb rather than kbps , does that suffice our purpose ?

changed this line in version 6 of the diff

we should not use printf, rather use glib logging functions, glib_debug, glib_log, etc.

Do I use g_debug for the printf or simply remove them because they were initially meant for debugging purpose ?

Either remove this, or move this to the netstats example, as this output is useful in an example, we have relevant data printed in examples.

changed this line in version 9 of the diff

We don't want to have print_* functions in public headers.

changed this line in version 5 of the diff

added 1 commit

• d5af9f3a - Return network stats as kb

Compare with previous version

added 1 commit

• dad596c6 - Remove functions for bytes to kb conversion

Compare with previous version

added 1 commit

• a43fbd28 - Add PCAP_LIBS variable to configure.ac

Compare with previous version

Would it be possible to either use a GArrayPtr or a GSList instead ?

@bdejean Sorry but this is not currently used , It's been a while since I changed this to GSList in another comment's follow-up.I've removed this from the header.

changed this line in version 10 of the diff

How is it allocated/deallocated ?

I'm using g_slice_new() here

Is it fine?

I'm talking about the ip_text field ? Maybe you could even make it an array instead of pointer.

I kept it as ptr just to not bring in sized allocations

But as we already know the ip size I can surely make the changes if you want :)

changed this line in version 14 of the diff

I've made the req. changes

I think that you need at least 40+1 bytes to store an IPv6 address, for example "fe80:0000:0000:0000:020f:bbff:feb8:f89e". So 30 is not enough.

You need to rename the field reserved0 in order to avoid breaking the ABI.

Could you please prefix all public symbols with glibtop_ ? Would it also be possible to prefix accordingly all public types ?

@antares could you please tell what functions are to be called by users and what is internal to libgtop ? The whole point of this file is to control what functions are publicly exported/visibled to libgtop users.

changed this line in version 10 of the diff

@bdejean you may now have a look at the current diff , it just has the public API exposed

Pls let me know your views!

OK, I'm not sure about the names, but the exposed interface is definitely smaller.

@robertroth if you open a 3.30 branch, I could already merge by server code, that would help not mixing topics.

@antares the changes are on master for the gtop server, and are also causing merge conflicts, it would be great if you could rebase to master.

changed this line in version 15 of the diff

Please use double

changed this line in version 8 of the diff

Please use foreach or set a free_func https://developer.gnome.org/glib/unstable/glib-Pointer-Arrays.html#g-ptr-array-new-with-free-func

changed this line in version 12 of the diff

Pls have a look at the latest commit,it's been updated there

double please :-)

changed this line in version 8 of the diff

For all these kind of structs that implement a RFC, I think we either need a packed attribute or simply a G_STATIC_ASSERT to validate the size, don't we ?

That's a time_t I think.

@bdejean can you pls clear a doubt of mine here,

I have actually assigned the tv_sec field present in the time_t struct to the last_packet_time that's why I've used int , but if you say that I should use time_t I shall change to that.

@antares I'm not really sure where it comes from, but if it's a struct timespec (=> man 2 nanosleep) then it's definitely a time_t.

changed this line in version 13 of the diff

Made the changes, pls have a look at the diff.

General question: is this code really specific ? Shouldn't it go to common instead ?

We can't move the following files to common because they are specific to the Linux fs:

• procopenfiles.c
• proc_inode_parser
• netsockets

But the packet parsing files can be moved to common , again because after parsing they make use of function calls defined in the above files , so indirectly it means that even those are getting more specific

In case we want to move the packet processing part to common/ , pls let me know , few slight changes will be good enough to separate the generic part of the code to common/

If the packet processing is generic enough, yes, it's definitely a start to move it common/

changed this line in version 18 of the diff

*pkt = *old_packet ?

changed this line in version 11 of the diff

ok for the g_slice.

About style: may be a simple if () would be more readable than all that code in that switch/case.

changed this line in version 11 of the diff

only used once, just drop it, that pointer stuff make it harder to read.

changed this line in version 11 of the diff

n is never used, is it ?

changed this line in version 11 of the diff

style: you shouldn't need to cast to gpointer because in C everything is freely convertible from and to void*

changed this line in version 11 of the diff

Maybe you want to memset(pkt, 0, sizeof pkt) the whole Packet first ?

changed this line in version 18 of the diff

Are these 2 casts safe ? Better use strtoull ?

changed this line in version 12 of the diff

&* ?

changed this line in version 12 of the diff

feof is only set only when you've reached EOF and you've attempted to read again. feof is only there to tell you what was the error you've encountered, not to detect the EOF itself. That's really important and applies to both C and C++. The canonical way would be:

while (fgets(...) != NULL) {

}

if (!feof()) {
  // fgets said that an error was found and yet it's not feof, so it's definitely an IO error that should be reported
}

changed this line in version 12 of the diff

This only matches '1' to '8'. Is that what you really want ? Please use g_ascii_isdigit(currentFile[0]) instead.

changed this line in version 11 of the diff

Fine

I think you should use at least an unsigned long instead with %lu format. Or even a guint64 and "socket:[" G_GUINT64_FORMAT "]\n".

changed this line in version 17 of the diff

Then you'll probably need to allocate something instead of packing ints into pointers.

changed this line in version 17 of the diff

symlink_name and slnk_buf need to be g_free.

changed this line in version 18 of the diff

You could even directly use g_slice_copy https://developer.gnome.org/glib/unstable/glib-Memory-Slices.html#g-slice-copy

Or simply change Packet_init to something like:

Packet* Packet_init(Packet* old)
{
  return g_slice_copy(sizeof *old, old);
}

As pointed out in this discussion thread I just removed this function and replace with what you had suggested above.

Which one would you prefer?

changed this line in version 11 of the diff

Anyways I find this approach much better , so I have changed to this one , pls confirm if this is fine .

I don't understand that if.

changed this line in version 9 of the diff

In case the number of processes have reduced in the next refresh cycle then to keep only the latest updated number of entries in the GArray , I'm deleting those entries which are beyond the range of the latest number of entries

But I've deleted this function because as per the discussions in other threads print_* functions shouldn't be exposed

Drop fname and simply give "/proc/..." as an argument.

changed this line in version 11 of the diff

Drop this file from the merge request. You have to do a make clean, make distclean, drop this file, and configure the project again in order to not have issues with autotools, but this is a generated file, and should not be in the git repo, it should be gitignored.

changed this line in version 16 of the diff

Indentation is wrong here.

changed this line in version 10 of the diff

Indentation is wrong here.

changed this line in version 10 of the diff

Commented-out code is not useful. Either uncomment or delete.

changed this line in version 18 of the diff

Commented out code is not something we need in the codebase. If do_refresh_s remains empty after removing the commented code, remove the method too.

I need to discuss about this file , because it is sth we used for the libgtop daemon on Linux but now that we aren't (for the time being) , can you pls tell me whether I drop the file or not.

If not , then this function will be using the functions defined in stats.c

We have the gtop Linux daemon code on master.

changed this line in version 17 of the diff

The same as with other commented-out code parts.

added 1 commit

• 775bbd1e - API should return bytes transferred

Compare with previous version

added 1 commit

• bde5f8d0 - Remove printf statements

Compare with previous version

added 1 commit

• 4b56ccac - Prefix glibtop to public API

Compare with previous version

added 1 commit

• 78ddc002 - Minor changes in some func defintions

Compare with previous version

added 1 commit

• 2b5d8076 - Use free function to free the DBus stats

Compare with previous version

Any objections against calling this glibtop_init_netstats? glibtop_init_setup doesn't help me finding out what it initializes.

changed this line in version 13 of the diff

I've changed this

added 2 commits

• 2842be7e - Rename glibtop_init_stats to glibgtop_init_netstats
• a2f1c697 - Use time_t wherever required

Compare with previous version

added 1 commit

• 0d6a8a3e - Replace ptr by array

Compare with previous version

added 94 commits

• 0d6a8a3e...35a0ba8e - 4 commits from branch GNOME:master
• 9f99a620 - Add directory traversal of /proc
• 7da5e029 - Add file parsing for /proc/net/tcp
• 7ef01db5 - Add parsing of network connections from /proc/net/ files
• 294204a4 - Add test for netsocket.c
• cbe605bf - Add hashTable to store inode and pid
• 6e672dd7 - Add test for proc_inode_parse.c
• c312da68 - Add glib version of dir traversal
• 6a1e9337 - dev_handles : Open pcap handles for network interfaces
• 04da947f - Test for checking the opened pcap handles for network devices
• d6351f17 - Change the dir name approach to GFile
• 62f0013c - GFile changes in the args
• 9d236bb3 - Add GError for error handling
• dfe21cb2 - Switched to GINT_TO_POINTER
• 1bb526d4 - Replace malloc with g_slice_new
• 5dd7deb2 - Minor changes in test file
• c4cb26d5 - Fix warning
• 707cb7ad - coding style fixes
• 1153f96d - Add Interface's local address
• 77e1876f - Add local address to interfaces
• 5a3431d6 - Add functions for manipulating packets
• 0967cbf5 - Add connections details to keep track of relevant packets
• 5e6947d7 - Add process details to track corresponding connections
• b47de8ef - Add callback for tcp
• 30a40f27 - Add more packet functions
• a82d6754 - Add hash table for socket details and inode
• 360a0ee4 - Add packet processing functions
• 3ff43963 - Modifications in packet function prototypes
• 99eb516b - Fix struct member name issues
• 5fb75c27 - Add structs for network stats output
• 744c3afe - Add main testing file
• ca2a700a - Redefinition issues fixed
• 7c0e3de4 - Add packet hash to packet struct
• b7b4406a - dev_handles.c : Modify packet processing functions
• de344641 - dev_handles.h: Add definition of packet headers
• 4f246f4b - connection: Modify the connection matching functions
• a7b8c23b - This commit has to be broken down into patches
• 54fe9773 - Separate the headers and tests
• b93f6157 - add missing files to Makefile
• 6bdb6b18 - Add Gio dependency
• 618052ac - Add Symbols to MakeFile
• 3afb310a - Add LIBGTOP prefix to header guards
• 210e3dc0 - Change Process_list to GSList
• cca275c7 - Change Conn_list to GSList
• 871a0a8e - Replace u_int data types by their GLib alternatives
• 9fc66c3a - Change Packet_list to GSList
• 6a2badf2 - Add new headers to the list of introspectables
• 07349930 - Change the network_stats_entry struct
• bc62a98a - Add network stats cmd to the daemon
• 9043f84d - Change flags to enable Daemon for Linux
• e7b29d7e - Define the libgtop_sysdeps_suid library for Linux
• 2ade4b58 - Add glibtop_init_p in Linux
• f75be015 - Change the local_addr field in struct glibtop_socket
• ee43fcd9 - Add glibtop_open_p function
• cd6555d8 - Remove the error logs
• 157ccc06 - Add DBus Interface to get the stats
• 2779c87a - Change the output GVariant
• 12200d78 - Add capture status function
• 2f33f6f9 - Add capture initializer function to DBus
• 0f55aaa9 - Change Daemon settings for Linux
• fa32655d - Move the DBus example to examples dir
• c5ff9227 - Add packet capture init function to stats
• 15ba350a - Add GPtrArray for the dbus_stats
• 79e2eec1 - Add parameter in the get_stats_instance invocation
• a28097ef - Change the condition for keeping the capture active
• 8901b39f - Rename the toggle_capture_status function
• 50e7f227 - WIP: Add the dictionary for DBus Messages
• 6b8b216d - Remove proc_name from various structs
• e399c2c0 - Fix the issue in dictionary in GVariants
• 638ba82e - Add g_timeout_add in the dbus interface
• a2a56534 - Fix timeout issue in DBus interface
• a9501139 - Add check to stop the packet capture
• 72bffccf - Fix segfault in GetStats
• 20df42f6 - Add config files for DBus
• 55559e8f - Add set and reset functions to dbus-interface
• 90fb9e3e - Discard values ~0.00 in the network stats
• adaad6be - Activate the packet capture in the network test example
• eeba585a - Use g_error wherever required
• 5a6817a1 - Fix warnings during compilation
• 3de94ded - Return network stats as kb
• 11334ceb - Remove functions for bytes to kb conversion
• d1c8b999 - Add PCAP_LIBS variable to configure.ac
• cfc41c86 - API should return bytes transferred
• 38a74f76 - Remove printf statements
• 464f62af - Prefix glibtop to public API
• ba1d787d - Minor changes in some func defintions
• 3bf1e71d - Use free function to free the DBus stats
• 764105e2 - Rename glibtop_init_stats to glibgtop_init_netstats
• 5acc0c5c - Use time_t wherever required
• c64a8159 - Replace ptr by array
• 896595d7 - Keep double in DBus Interface

Compare with previous version

added 2 commits

• 9e29402e - Delete config.rpath
• 1b047597 - Delete the stale packets

Compare with previous version

Hi folks, just passing by to ask whether there is something blocking this implementation/review that I could help. We would love to land this feature in Usage for 3.30, so count with me.

I only have two more unresolved discussions, both of them being nitpicking on the installation path (dbus_interface being in examples dir,as a noinst executable, the other one being hardcoded ~/.cache/jhbuild/build/libgtop/examples/dbus_interface in the dbus service file), those should be resolved for obvious reasons. The other 8 unresolved discussions (1 request for explanation = more comments, 7 suggestions) are suggestions for improvements (e.g. too short char array, memset missing), all of them make sense.

@felipeborges @robertroth I had my final exams until last week and am currently mid way traveling a bit , I will look into this asap once I get back in a day or two.

added 1 commit

• a90417d9 - Fix warnings

Compare with previous version

added 1 commit

• 0529953b - Move packet processing files to common

Compare with previous version