Package: cpqarrayd Version: 2.0 Followup-For: Bug #295189 Tag: patch Hi,
I went over the various malloc() calls in the source to see if anything could be improved. Apparently it doesn't free() anything, ever (except for one piece of memory which it allocates but never uses - wtf). Most of the recurring malloc calls could be removed in favour of using a bit of stack. Some others happen only at system startup. A number of those were really written out versions of strdup() - I changed those to real strdup() calls to improve readability. I did some cursory testing and this version seems to work but more thorough testing would be welcome, especially for the SNMP bits (which we don't use - yet). Time will tell if all memory leaks have been plugged. Regards, -- Wessel Dankers <[EMAIL PROTECTED]> â UNIX system administration Universiteit van Tilburg â IT Services â Postbus 90153, 5000 LE Bezoekadres > Warandelaan 2 â Telefoon 013 466 3520 â www.uvt.nl
diff -urN cpqarrayd-2.0,old/cciss_functions.c cpqarrayd-2.0/cciss_functions.c
--- cpqarrayd-2.0,old/cciss_functions.c 2003-09-24 18:05:25.000000000 +0200
+++ cpqarrayd-2.0/cciss_functions.c 2005-04-11 11:18:30.373715000 +0200
@@ -67,7 +67,7 @@
{
int result, outfile;
IOCTL_Command_struct iocommand;
- unsigned char *buffer;
+ unsigned char buffer[128];
iocommand.LUN_info.LunAddrBytes[0] = 0;
iocommand.LUN_info.LunAddrBytes[1] = 0;
@@ -98,7 +98,6 @@
iocommand.Request.CDB[10] = 0x0; /* reserved, leave 0 */
iocommand.Request.CDB[11] = 0x0; /* control ? */
- buffer = (unsigned char *) malloc (128);
memset (buffer, 0x0, 128);
iocommand.buf_size = 128;
iocommand.buf = buffer;
@@ -125,7 +124,7 @@
{
int result, outfile;
IOCTL_Command_struct iocommand;
- unsigned char *buffer;
+ unsigned char buffer[512];
iocommand.LUN_info.LunAddrBytes[0] = 0;
iocommand.LUN_info.LunAddrBytes[1] = 0;
@@ -157,7 +156,6 @@
iocommand.Request.CDB[11] = 0x0;
iocommand.Request.CDB[12] = 0x0;
- buffer = (unsigned char *) malloc (512);
memset (buffer, 0x0, 512);
iocommand.buf_size = 512;
iocommand.buf = buffer;
diff -urN cpqarrayd-2.0,old/cpqarrayd.c cpqarrayd-2.0/cpqarrayd.c
--- cpqarrayd-2.0,old/cpqarrayd.c 2005-04-11 13:12:32.536925362 +0200
+++ cpqarrayd-2.0/cpqarrayd.c 2005-04-11 12:52:27.168263000 +0200
@@ -45,6 +45,10 @@
#include <errno.h>
+#ifndef HOST_NAME_MAX
+#define HOST_NAME_MAX 255
+#endif
+
#include "cpqarrayd.h"
#include "discover.h"
#include "status.h"
@@ -141,8 +145,7 @@
case 't':
if (opts.nr_traphosts < 10) {
/* strlen doesn't count terminating \0. Add one to fix that. */
- opts.traphosts[opts.nr_traphosts] = (char
*)malloc(strlen(optarg)+1);
- strncpy(opts.traphosts[opts.nr_traphosts], optarg,
strlen(optarg)+1);
+ opts.traphosts[opts.nr_traphosts] = strdup(optarg);
opts.nr_traphosts++;
}
else {
@@ -177,8 +180,11 @@
}
/* get ip of current machine for traps */
- buffer = (char *)malloc(50);
- if (gethostname(buffer, 50) == 0) {
+ buffer = (char *)malloc(HOST_NAME_MAX + 1);
+ buffer[0] = '\0';
+ if (gethostname(buffer, HOST_NAME_MAX + 1) == 0) {
+ /* It is unspecified whether a truncated hostname will be NUL-terminated.
*/
+ buffer[HOST_NAME_MAX] = '\0';
myhost = gethostbyname(buffer);
myip = ((unsigned char) myhost->h_addr_list[0][3] << 24) +
((unsigned char) myhost->h_addr_list[0][2] << 16) +
@@ -187,6 +193,8 @@
}
else {
perror("gethostname");
+ strncpy(buffer, "(none)", HOST_NAME_MAX);
+ buffer[HOST_NAME_MAX] = '\0';
}
/* test for trap destinations */
@@ -238,11 +246,11 @@
/* END OF ADDITIONAL CODE */
}
- buffer = (char *)malloc(1024);
+ /* buffer = (char *)malloc(1024); */
/* sprintf (buffer, "cpqarrayd[%d]\0", getpid); */
openlog ("cpqarrayd", LOG_CONS, LOG_USER);
syslog(LOG_INFO, "Logging Enabled...");
- free(buffer);
+ /* free(buffer); */
while (keeprunning) {
status_check(opts);
diff -urN cpqarrayd-2.0,old/discover.c cpqarrayd-2.0/discover.c
--- cpqarrayd-2.0,old/discover.c 2005-04-11 13:12:32.537925217 +0200
+++ cpqarrayd-2.0/discover.c 2005-04-11 11:38:35.877160000 +0200
@@ -141,9 +141,7 @@
boardid2str (io.c.id_ctlr.board_id, buffer);
- ctrls_found[ctrls_found_num].ctrl_devicename = (char
*)malloc(strlen(buffer)+1);
- strncpy (ctrls_found[ctrls_found_num].ctrl_devicename,
- buffer, strlen(buffer));
+ ctrls_found[ctrls_found_num].ctrl_devicename = strdup(buffer);
ctrls_found[ctrls_found_num].ctrl_type = CTRLTYPE_IDA;
@@ -160,8 +158,7 @@
if (opts.verbose) printf(" Found a %s (%d Logical drives)\n", buffer,
ctrls_found[ctrls_found_num].num_logd_found);
- ctrls_found[ctrls_found_num].devicefile = (char *)malloc(strlen(devicefile));
- strcpy(ctrls_found[ctrls_found_num].devicefile, devicefile);
+ ctrls_found[ctrls_found_num].devicefile = strdup(devicefile);
close (devicefd);
@@ -299,12 +296,8 @@
listlength |= (0xff & (unsigned int)(logicalluns.LUNlist_len[2])) << 8;
listlength |= (0xff & (unsigned int)(logicalluns.LUNlist_len[3]));
- ctrls_found[ctrls_found_num].ctrl_devicename = (char *)malloc(17);
- strncpy (ctrls_found[ctrls_found_num].ctrl_devicename,
- "CCISS Controller", 16);
- ctrls_found[ctrls_found_num].ctrl_devicename[16] = 0x0;
- ctrls_found[ctrls_found_num].devicefile = (char *)malloc(strlen(devicefile));
- strcpy(ctrls_found[ctrls_found_num].devicefile, devicefile);
+ ctrls_found[ctrls_found_num].ctrl_devicename = strdup("CCISS Controller");
+ ctrls_found[ctrls_found_num].devicefile = strdup(devicefile);
ctrls_found[ctrls_found_num].ctrl_type = CTRLTYPE_CCISS;
ctrls_found[ctrls_found_num].num_logd_found = listlength / 8;
diff -urN cpqarrayd-2.0,old/sendtrap.c cpqarrayd-2.0/sendtrap.c
--- cpqarrayd-2.0,old/sendtrap.c 2003-09-26 13:41:11.000000000 +0200
+++ cpqarrayd-2.0/sendtrap.c 2005-04-11 12:54:19.730961000 +0200
@@ -52,7 +52,7 @@
#ifdef HAVE_SNMPTRAP
struct snmp_session session, *ss;
struct snmp_pdu *pdu;
- char *statusmsg;
+ char statusmsg[16];
oid enterprise[] = {1,3,6,1,4,1,300};
oid statusoid[] = {1,3,6,1,4,1,300,1};
oid messageoid[] = {1,3,6,1,4,1,300,2};
@@ -65,11 +65,8 @@
snmp_sess_init( &session );
- /* strlen() doesn't count the terminating \0, so we do +1 in malloc. */
- session.peername = (char *)malloc(strlen(opts.traphosts[counter])+1);
- strcpy (session.peername, opts.traphosts[counter]);
- session.community = (char *)malloc(strlen(community)+1);
- strcpy (session.community, community);
+ session.peername = strdup(opts.traphosts[counter]);
+ session.community = strdup(community);
session.community_len = strlen(session.community);
session.version = SNMP_VERSION_1;
session.retries = 5;
@@ -94,13 +91,11 @@
pdu->contextEngineID = 0x0;
*pdu_in_addr_t = get_myaddr();
- statusmsg = (char *)malloc(12);
sprintf(statusmsg, "%d", status);
snmp_add_var (pdu, statusoid, sizeof(statusoid) / sizeof (oid), 'i',
statusmsg);
- messagebuf = (char *)malloc(strlen(message)+1);
- strcpy(messagebuf,message);
+ messagebuf = strdup(message);
snmp_add_var (pdu, messageoid, sizeof(messageoid) / sizeof (oid), 's',
message);
diff -urN cpqarrayd-2.0,old/status.c cpqarrayd-2.0/status.c
--- cpqarrayd-2.0,old/status.c 2003-09-26 13:41:11.000000000 +0200
+++ cpqarrayd-2.0/status.c 2005-04-11 11:43:03.481377000 +0200
@@ -50,7 +50,7 @@
ida_ioctl_t io, io2;
int status, nr_blks, blks_tr, trap_stat;
float pvalue;
- char *statusmsg;
+ char statusmsg[1024];
int counter;
@@ -126,7 +126,6 @@
ctrls_found[ctrl_cntr].log_disk[logd_cntr].status, status,
pvalue);
}
- statusmsg = (char *)malloc(1024);
sprintf(statusmsg, statusstr[status], ctrl_cntr, logd_cntr, pvalue);
if (opts.debug) {
printf("DEBUG: sending traps.\n");
@@ -156,7 +155,6 @@
ctrls_found[ctrl_cntr].log_disk[logd_cntr].pvalue,
pvalue);
}
- statusmsg = (char *)malloc(1024);
sprintf(statusmsg, statusstr[status], ctrl_cntr, logd_cntr, pvalue);
if (opts.debug) {
printf("DEBUG: sending traps.\n");
@@ -191,7 +189,7 @@
ida_ioctl_t io, io2;
int status, nr_blks, blks_tr, trap_stat;
float pvalue;
- char *statusmsg;
+ char statusmsg[2048];
int counter;
cciss_event_type event;
@@ -204,7 +202,6 @@
}
devicefd = open (ctrls_found[ctrl_cntr].devicefile, O_RDONLY);
- statusmsg = (char *)malloc(2048);
result = cciss_get_event(devicefd, 0, &event);
while (!CompareEvent(event,0,0,0)) {
signature.asc
Description: Digital signature
