From owner-freebsd-firewire@FreeBSD.ORG Wed Aug 6 01:38:20 2008 Return-Path: Delivered-To: freebsd-firewire@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 0407B106566B; Wed, 6 Aug 2008 01:38:20 +0000 (UTC) (envelope-from freebsd@sopwith.solgatos.com) Received: from parsely.rain.com (parsely.rain.com [199.26.172.196]) by mx1.freebsd.org (Postfix) with ESMTP id 62BFE8FC0A; Wed, 6 Aug 2008 01:38:19 +0000 (UTC) (envelope-from freebsd@sopwith.solgatos.com) Received: from sopwith.solgatos.com (uucp@localhost) by parsely.rain.com (8.11.4/8.11.4) with UUCP id m7610Uv13916; Tue, 5 Aug 2008 18:00:30 -0700 (PDT) (envelope-from freebsd@sopwith.solgatos.com) Received: from localhost by sopwith.solgatos.com (8.8.8/6.24) id AAA24024; Wed, 6 Aug 2008 00:59:23 GMT Message-Id: <200808060059.AAA24024@sopwith.solgatos.com> To: Sean Bruno In-reply-to: Your message of "Tue, 01 Jul 2008 17:26:27 PDT." <486ACB33.50204@miralink.com> Date: Tue, 05 Aug 2008 17:59:23 +0100 From: Dieter Cc: freebsd-firewire@freebsd.org Subject: Re: This is where I'm going with fwcontrol X-BeenThere: freebsd-firewire@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Firewire support in FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Aug 2008 01:38:20 -0000 In message <486ACB33.50204@miralink.com>, Sean Bruno writes: > > Here is a current diff against RELENG_7 which seems to be the same as > > -CURRENT. fwcontrol.c: In function 'main': fwcontrol.c:726: warning: comparison is always false due to limited range of data type I changed priority_budget from int to long, is this the correct fix? BUG: + if (adjust_gap_count) + send_phy_config(fd, adjust_gap_count, -1); Since I need to set this to 0 (see PR 113785), the if() fails and doesn't execute the send_phy_config(). I fixed this by adding a seperate flag, need_to_send_adjust_gap_count. Please feel free to think up a better name for the flag. :-) NOTE: some of the other options have the same problem, if the argument can be 0. The patch below only fixes -f. I did add some error checking, mostly for malloc. With the patch below, -f, -r, -u, -S work for me. FreeBSD 7.0 on AMD64, with the NEC fw controller. I haven't tested -R yet, I suspect the VIA fw controller still doesn't work. =================================================================== RCS file: RCS/fwcontrol.c,v retrieving revision 1.3 diff -u -r1.3 fwcontrol.c --- fwcontrol.c 2008/08/05 23:16:45 1.3 +++ fwcontrol.c 2008/08/06 00:33:25 @@ -97,7 +97,7 @@ get_dev(int fd, struct fw_devlstreq *data) { if (data == NULL) - err(1, "malloc"); + err(1, "arg data is NULL"); /* probably due to failure of malloc in calling function */ if( ioctl(fd, FW_GDEVLST, data) < 0) { err(1, "ioctl"); } @@ -188,6 +188,8 @@ u_int32_t *qld, res; asyreq = (struct fw_asyreq *)malloc(sizeof(struct fw_asyreq_t) + 16); + if (asyreq == NULL) + err(1, "malloc"); asyreq->req.len = 16; #if 0 asyreq->req.type = FWASREQNODE; @@ -226,6 +228,8 @@ struct fw_asyreq *asyreq; asyreq = (struct fw_asyreq *)malloc(sizeof(struct fw_asyreq_t) + 12); + if (asyreq == NULL) + err(1, "malloc"); asyreq->req.len = 12; asyreq->req.type = FWASREQNODE; asyreq->pkt.mode.ld[0] = 0; @@ -251,6 +255,8 @@ struct fw_asyreq *asyreq; asyreq = (struct fw_asyreq *)malloc(sizeof(struct fw_asyreq_t) + 12); + if (asyreq == NULL) + err(1, "malloc"); asyreq->req.len = 12; asyreq->req.type = FWASREQNODE; asyreq->pkt.mode.common.tcode = FWTCODE_PHY; @@ -268,6 +274,8 @@ struct fw_asyreq *asyreq; asyreq = (struct fw_asyreq *)malloc(sizeof(struct fw_asyreq_t) + 16); + if (asyreq == NULL) + err(1, "malloc"); asyreq->req.len = 16; asyreq->req.type = FWASREQNODE; asyreq->pkt.mode.wreqq.dst = FWLOCALBUS | (node & 0x3f); @@ -643,7 +651,11 @@ err(1, "ioctl FW_SRSTREAM"); buf = (char *)malloc(1024*16); + if (buf == NULL) + err(1, "malloc"); len = read(fd, buf, 1024*16); + if (len == -1) + err(1, "read"); ptr = (u_int32_t *) buf; ciph = (struct ciphdr *)(ptr + 1); @@ -688,12 +700,13 @@ * to iterate through */ int display_board_only = 0; - int priority_budget = 0; + long priority_budget = 0; int display_crom = 0; char *crom_string = NULL; char *crom_string_hex = NULL; int display_crom_hex = 0; int adjust_gap_count = 0; + int need_to_send_adjust_gap_count = 0; int reset_gap_count = 0; int load_crom_from_file = 0; int set_fwmem_target = 0; @@ -731,6 +744,8 @@ break; case 'c': crom_string = malloc(strlen(optarg)+1); + if (crom_string == NULL) + err(1, "malloc"); strcpy(crom_string, optarg); display_crom = 1; open_needed = 1; @@ -739,6 +754,8 @@ break; case 'd': crom_string_hex = malloc(strlen(optarg)+1); + if (crom_string == NULL) + err(1, "malloc"); strcpy(crom_string_hex, optarg); display_crom_hex = 1; open_needed = 1; @@ -747,6 +764,7 @@ break; case 'f': adjust_gap_count = strtol(optarg, NULL, 0); + need_to_send_adjust_gap_count = 1; open_needed = 1; command_set = 1; display_board_only = 0; @@ -827,6 +845,8 @@ break; case 'R': recv_data = malloc(strlen(optarg)+1); + if (recv_data == NULL) + err(1, "malloc"); strcpy(recv_data, optarg); open_needed = 1; command_set = 1; @@ -834,6 +854,8 @@ break; case 'S': send_data = malloc(strlen(optarg)+1); + if (send_data == NULL) + err(1, "malloc"); strcpy(send_data, optarg); open_needed = 1; display_board_only = 0; @@ -904,8 +926,10 @@ /* * Adjust the gap count for this card/bus to value "-f" */ - if (adjust_gap_count) + if (need_to_send_adjust_gap_count) + { send_phy_config(fd, adjust_gap_count, -1); + } /* * Reset the gap count for this card/bus "-g"