From owner-freebsd-firewire@FreeBSD.ORG Sun Aug 31 19:37:14 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 59087106564A for ; Sun, 31 Aug 2008 19:37:14 +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 BD69A8FC08 for ; Sun, 31 Aug 2008 19:37:13 +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 m7VJauv90290; Sun, 31 Aug 2008 12:36:56 -0700 (PDT) (envelope-from freebsd@sopwith.solgatos.com) Received: from localhost by sopwith.solgatos.com (8.8.8/6.24) id TAA10392; Sun, 31 Aug 2008 19:34:34 GMT Message-Id: <200808311934.TAA10392@sopwith.solgatos.com> To: Sean Bruno In-reply-to: Your message of "Fri, 29 Aug 2008 14:18:11 PDT." <48B86793.6080404@miralink.com> Date: Sun, 31 Aug 2008 12:34:34 +0100 From: Dieter Cc: freebsd-firewire@freebsd.org Subject: Re: New and improved? patch 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: Sun, 31 Aug 2008 19:37:14 -0000 >> Freshly updated with new found information on what the heck -f -g >> actually do! >> >> See how this behaves for you. usage: > [-f force_root ] Suggestion: [-f force_root_node ] or maybe "desired_root_node" or maybe "root_node" or maybe just "node", same as -c -d -o -s > -g: broadcast gap_count by phy_config packet Suggestion: -g: set gap_count by broadcasting phy_config packet or maybe just: -g: set gap_count > -f: broadcast force_root by phy_config packet Suggestion: -f force a node to become the root node by broadcasting phy_config packet or maybe just: -f force a node to become the root node 7.0 AMD64 # ./fwcontrol -f -5 fwcontrol: main:set_root_node out of range: No such file or directory "No such file or directory" seems wrong 7.0 AMD64 # ./fwcontrol -g 70 fwcontrol: main:set_gap_count out of range: No such file or directory "No such file or directory" seems wrong 7.0 AMD64 # ./fwcontrol -d 70 fwcontrol: no such node -1. (a) 70 becomes -1 ? (b) Wouldn't -d have the same range as -f ? 7.0 AMD64 # ./fwcontrol -c 70 fwcontrol: no such node -1. (a) 70 becomes -1 ? (b) Wouldn't -c have the same range as -f ? 7.0 AMD64 # ./fwcontrol -o 70 fwcontrol: main: node out of range: 70 : No such file or directory "No such file or directory" seems wrong 7.0 AMD64 # ./fwcontrol -s 70 fwcontrol: main: node out of range: 70 : No such file or directory "No such file or directory" seems wrong - asyreq->pkt.mode.ld[1] |= (root_node & 0x3f) << 24 | 1 << 23; + asyreq->pkt.mode.ld[1] |= ((root_node << 24) | (1 << 23)); if (gap_count >= 0) - asyreq->pkt.mode.ld[1] |= 1 << 22 | (gap_count & 0x3f) << 16; + asyreq->pkt.mode.ld[1] |= ((1 << 22) | (gap_count << 16)); Any reason for pulling out the "& 0x3f" ? Yeah it should be redundant now that there is range checking on the arguments, but as you said, fwcontrol is dangerous and the mask makes it safer at very little cost. - for (i = 0; i < 4; i++) { - snprintf(name, sizeof(name), "%s.%d", devbase, i); - if ((*fd = open(name, O_RDWR)) >= 0) - break; - } + *fd = open(devname, O_RDWR); Looking at various firewire man pages, I don't find any explanation of the various /dev filenames, such as what the .%d part was/is for. So I have no clue why this code was changed. Did I miss a discussion? From owner-freebsd-firewire@FreeBSD.ORG Sun Aug 31 20:06:42 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 71ACD10656BE for ; Sun, 31 Aug 2008 20:06:42 +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 98A938FC1A for ; Sun, 31 Aug 2008 20:06:40 +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 m7VK6W390340; Sun, 31 Aug 2008 13:06:32 -0700 (PDT) (envelope-from freebsd@sopwith.solgatos.com) Received: from localhost by sopwith.solgatos.com (8.8.8/6.24) id UAA08072; Sun, 31 Aug 2008 20:05:44 GMT Message-Id: <200808312005.UAA08072@sopwith.solgatos.com> In-reply-to: Your message of "Sun, 31 Aug 2008 12:34:34 BST." Date: Sun, 31 Aug 2008 13:05:44 +0100 From: Dieter Cc: freebsd-firewire@freebsd.org Subject: Re: New and improved? patch 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: Sun, 31 Aug 2008 20:06:42 -0000 > 7.0 AMD64 # ./fwcontrol -f -5 > fwcontrol: main:set_root_node out of range: No such file or directory > > "No such file or directory" seems wrong err(EX_USAGE, "%s:set_root_node out of range", __func__); Err() is correct for places where errno would be set, such as checking the return code from read(2). But for the range checks, errno does not apply, so err() gives misleading results. The err(3) man page isn't clear, but it looks like errx(3) is the function you want for the range checks. Changing err() to errx() gives: 7.0 AMD64 # ./fwcontrol -f 70 fwcontrol: main:set_root_node out of range From owner-freebsd-firewire@FreeBSD.ORG Mon Sep 1 11:06:54 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 9E05A1065687 for ; Mon, 1 Sep 2008 11:06:54 +0000 (UTC) (envelope-from owner-bugmaster@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 742738FC28 for ; Mon, 1 Sep 2008 11:06:54 +0000 (UTC) (envelope-from owner-bugmaster@FreeBSD.org) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.2/8.14.2) with ESMTP id m81B6sKq068408 for ; Mon, 1 Sep 2008 11:06:54 GMT (envelope-from owner-bugmaster@FreeBSD.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.2/8.14.1/Submit) id m81B6rBr068404 for freebsd-firewire@FreeBSD.org; Mon, 1 Sep 2008 11:06:53 GMT (envelope-from owner-bugmaster@FreeBSD.org) Date: Mon, 1 Sep 2008 11:06:53 GMT Message-Id: <200809011106.m81B6rBr068404@freefall.freebsd.org> X-Authentication-Warning: freefall.freebsd.org: gnats set sender to owner-bugmaster@FreeBSD.org using -f From: FreeBSD bugmaster To: freebsd-firewire@FreeBSD.org Cc: Subject: Current problem reports assigned to freebsd-firewire@FreeBSD.org 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: Mon, 01 Sep 2008 11:06:54 -0000 Current FreeBSD problem reports Critical problems Serious problems S Tracker Resp. Description -------------------------------------------------------------------------------- o kern/74238 firewire [firewire] fw_rcv: unknown response; firewire ad-hoc w 1 problem total. Non-critical problems S Tracker Resp. Description -------------------------------------------------------------------------------- o kern/113785 firewire [firewire] dropouts when playing DV on firewire 1 problem total. From owner-freebsd-firewire@FreeBSD.ORG Sat Sep 6 22:23:18 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 B75AA1065671 for ; Sat, 6 Sep 2008 22:23:18 +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 4E3518FC14 for ; Sat, 6 Sep 2008 22:23:17 +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 m86MNCI08555; Sat, 6 Sep 2008 15:23:12 -0700 (PDT) (envelope-from freebsd@sopwith.solgatos.com) Received: from localhost by sopwith.solgatos.com (8.8.8/6.24) id WAA13366; Sat, 6 Sep 2008 22:22:07 GMT Message-Id: <200809062222.WAA13366@sopwith.solgatos.com> To: Sean Bruno , freebsd-firewire@freebsd.org In-reply-to: Your message of "Sun, 31 Aug 2008 13:05:44 BST." Date: Sat, 06 Sep 2008 15:22:07 +0100 From: Dieter Cc: Subject: Re: New and improved? patch [ 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: Sat, 06 Sep 2008 22:23:18 -0000 > > 7.0 AMD64 # ./fwcontrol -f -5 > > fwcontrol: main:set_root_node out of range: No such file or directory > > > > "No such file or directory" seems wrong > > err(EX_USAGE, "%s:set_root_node out of range", __func__); > > Err() is correct for places where errno would be set, such as > checking the return code from read(2). But for the range checks, > errno does not apply, so err() gives misleading results. > > The err(3) man page isn't clear, but it looks like errx(3) is > the function you want for the range checks. > > Changing err() to errx() gives: > > 7.0 AMD64 # ./fwcontrol -f 70 > fwcontrol: main:set_root_node out of range Also, I'm thinking we probably don't want "\n" in the err() and errx() calls, since they put that in automagically. When a file cannot be opened, it is useful to print the name of the file. int len=1024, i; if ((file = fopen(filename, "r")) == NULL) - err(1, "load_crom"); + err(1, "load_crom filename = %s", filename); for (i = 0; i < len/(4*8); i ++) { fscanf(file, DUMP_FORMAT, p, p+1, p+2, p+3, p+4, p+5, p+6, p+7); if (open_dev(&fd, devbase) < 0) { - errx(EX_IOERR, "%s: Error opening board #%d\n", __func__, current_board); + err(EX_IOERR, "%s: Error opening firewire controller #%d %s ", + __func__, current_board, devbase); } } /* Since open_dev() calls open(2), which sets errno, this time we want err(3) rather than errx(3).