Network #44
Network #44
Conversation
|
network monitoring |
There was a problem hiding this comment.
Hi Mirek, here are a few comments and high-level questions about this PR. This module doesn't really have a maintainer at the moment, but as a representative of a site that is using it I'm hoping it will get some more attention which I think it badly needs.
Thanks for wanting to make it better!
- Andrew
configure/RELEASE
Outdated
| MAKE_TEST_IOC_APP=YES | ||
|
|
||
| #MAKE_TEST_IOC_APP=YES | ||
| MAKE_TEST_NET_IOC_APP=YES |
There was a problem hiding this comment.
The configure/RELEASE file is only supposed to contain variables with paths pointing to other modules. I know you didn't start this, but you're changing it so please move this variable to the configure/CONFIG_SITE file where it really belongs.
| /* #if HAVE_CONFIG_H | ||
| #include "config.h" | ||
| #endif | ||
|
|
There was a problem hiding this comment.
There is a large block of commented-out #include statements being added starting here. If they aren't required please don't add them to the code; if they are they shouldn't be commented out...
| { NULL, 0.0 }, | ||
| }; | ||
|
|
||
| #ifdef WITH_NETWORK_SUPPORT | ||
| /* add here network support */ | ||
| #endif |
There was a problem hiding this comment.
Is there something missing here?
| @@ -692,7 +1005,7 @@ static double minMBuf(int pool) | |||
| int i = 0; | |||
| double lowest = 1.0, comp; | |||
|
|
|||
| while ((i < CLUSTSIZES) && (clustinfo[pool][i][0] != 0)) | |||
| while ((clustinfo[pool][i][0] != 0) && (i < CLUSTSIZES)) | |||
There was a problem hiding this comment.
I would normally expect an index check to be done before actually using the index (so you don't try to read from storage that you don't own), can you explain why you swapped these around?
| extern struct dbBase *pdbbase; | ||
|
|
||
| long ioccar(char *precordname,int level, int *Pcal, int *Pcalnconn, int *Pcaldconn) | ||
| { |
There was a problem hiding this comment.
This routine looks to be doing almost the same thing as the dbcaStats() provided by the IOC (see dbCaTest.h) — is it really necessary to duplicate it instead of just calling that? The code here also needs careful examination because it hasn't had all of its printf() calls replaced properly...
| printf("\t\ttx_packets = %10u; rx_packets = %10u\n" | ||
| "\t\ttx_bytes = %10u; rx_bytes = %10u\n" | ||
| "\t\ttx_dropped = %10u; rx_dropped = %10u\n" | ||
| "\t\tmulticast = %10u; collisions = %10u\n", |
There was a problem hiding this comment.
More commented-out code, not really suitable for merging.
| @@ -0,0 +1,301 @@ | |||
| record(stringin, "$(IOCNAME):STARTTOD") | |||
| { | |||
There was a problem hiding this comment.
I'm seeing duplication of records here, can you explain if and how these are supposed to differ? For example there's an $(IOCNAME):STARTTOD record in the ioc.template file which the build instantiates into several different installed DB files.
|
Hi @mirek23 - I am looking to take over responsibility for the iocStats module. I see here that @anjohnson has a few comments for you, do you have any updates to this pull request? |
… for 20 sec time intervals with the respect to the previous cycle.
New features added: