From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-00069f02.pphosted.com (mx0a-00069f02.pphosted.com [205.220.165.32]) by sourceware.org (Postfix) with ESMTPS id 986C838582B4 for ; Sun, 30 Oct 2022 07:37:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 986C838582B4 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=oracle.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=oracle.com Received: from pps.filterd (m0246629.ppops.net [127.0.0.1]) by mx0b-00069f02.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 29TKK0vI020445; Sun, 30 Oct 2022 07:37:12 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=message-id : date : subject : to : references : from : cc : in-reply-to : content-type : content-transfer-encoding : mime-version; s=corp-2022-7-12; bh=vXgX7qk7/R17tdKu8ISL+V26p77WUwJq9dV1EJ2kluQ=; b=MmWxgph5AWrmTv8Tw/W49yh+6qGpd1YRObN+/Z4CrKdV2gJDGtwPLZVAdcIdQoafw6Ob QIHH3nfCmBnlWb269hAF71P+9fbTGvzFHetljlynDHDQHr62R7TMGgy1+O+/kXuRiYy0 5aJok0eDoOGoIEkKBbvBdgFJ+Gr0fEY81k9TEZO8ILoYRFlEoil6oMELpxgcck7i1G6/ pdaXb2PHpcIFMlMIS9cQmbPEIBB6qv5R68aCmJJl2lnAJBjiHj3pI5NHbqCMpOUTR6y2 vlyKCYLAMVf9RvBQS5sOsjQwTNFvvmfco+IWVC6Py8VRjZTwdEDHC6Q42MXreuB+Xjsm dQ== Received: from phxpaimrmta03.imrmtpd1.prodappphxaev1.oraclevcn.com (phxpaimrmta03.appoci.oracle.com [138.1.37.129]) by mx0b-00069f02.pphosted.com (PPS) with ESMTPS id 3kgv2a99hn-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Sun, 30 Oct 2022 07:37:12 +0000 Received: from pps.filterd (phxpaimrmta03.imrmtpd1.prodappphxaev1.oraclevcn.com [127.0.0.1]) by phxpaimrmta03.imrmtpd1.prodappphxaev1.oraclevcn.com (8.17.1.5/8.17.1.5) with ESMTP id 29U71r5B029127; Sun, 30 Oct 2022 07:37:11 GMT Received: from nam04-dm6-obe.outbound.protection.outlook.com (mail-dm6nam04lp2043.outbound.protection.outlook.com [104.47.73.43]) by phxpaimrmta03.imrmtpd1.prodappphxaev1.oraclevcn.com (PPS) with ESMTPS id 3kgtm28nn3-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Sun, 30 Oct 2022 07:37:11 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=nWgVtgwLORXtyNBZWdYijkroQk180xQgxBUfOXGN44nvwju4va/vcm7ngbnzlpH5mCSViN+nl7GYYjC7jNQjEiwtMLIsRT3xO4Z0xsfBdBRpXyJKlzFE7a9eCOMEra43eHnXS826EkUGw5XbJ6p9lBRM4wybVbkRSK2heSK80hI6+H7NUE6tn9etSAVnM6W10eMroZTLK8lhDJs25y8qDCUapLbDvw2dj/DLTj9fsJJ8pQlIR1dTWpuK8soHgFJ0TUol5K7Ga5nkJpfQBC9njdL30uIc20ksj2uEH8czZHXjo/uTAmOkKWxtK0ZLuCT1qScRE9DoCP/4kE++7xwuUg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=vXgX7qk7/R17tdKu8ISL+V26p77WUwJq9dV1EJ2kluQ=; b=Z0pKh85tUOpouzNGdqiZZQxXWv/w1JEU/I1HaG5tVmX0yrZYlAfDlnaSmAB5Fa8xG5uobyF2rHypu2I3UPXTUGyVpvjHGQBt12PSjmbyd3ZU8XCODDioLA08Lnn8NSv94HTFOoL7i7kikQjIlaDTikn6rqgqUeutmHsUJqSyz5mysoSrYE9hU2Oat11fGePf8mimGBf4vTSUJFm0/jmJ+PU1GxTmV7WA8h38IC0P3N3rqLOdSxj0qjJIjfMUytXPRcx5/rqso5KhtW9XrM43naHnNkKiISdQ7ZGjzDAUIJvKdZ17X/cyTrVU+vGdHLor9J2+KrCys8FquinQ5iKZ9w== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=oracle.com; dmarc=pass action=none header.from=oracle.com; dkim=pass header.d=oracle.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.onmicrosoft.com; s=selector2-oracle-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=vXgX7qk7/R17tdKu8ISL+V26p77WUwJq9dV1EJ2kluQ=; b=hvdGTVpcYaWXFtCfG1lH9wrmj5a3721Avfk6V1+nA+rH6Er456+sRFTr+xvYS/QiKquL8vOdkmkLwrTdl5epXgncpVj9ajyBtSdSt6PVcOALKuxJTxCAXhUCJxEsiKcQRz7K1fyr/jO0zdhAq2otfULNaXLQPx7v56k2o5ZlqfU= Received: from MWHPR1001MB2158.namprd10.prod.outlook.com (2603:10b6:301:2d::17) by BN0PR10MB4871.namprd10.prod.outlook.com (2603:10b6:408:128::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5769.18; Sun, 30 Oct 2022 07:37:09 +0000 Received: from MWHPR1001MB2158.namprd10.prod.outlook.com ([fe80::a505:15c2:a248:efa2]) by MWHPR1001MB2158.namprd10.prod.outlook.com ([fe80::a505:15c2:a248:efa2%7]) with mapi id 15.20.5723.033; Sun, 30 Oct 2022 07:37:09 +0000 Message-ID: <4905532c-9e02-8d07-0898-c94d61890f2f@oracle.com> Date: Sun, 30 Oct 2022 00:37:07 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.1 Subject: Re: [PATCH,V2 05/15] libsframe: add the SFrame library Content-Language: en-US To: Mike Frysinger References: <20221017221612.495324-1-indu.bhagat@oracle.com> <20221017221612.495324-6-indu.bhagat@oracle.com> From: Indu Bhagat Cc: binutils@sourceware.org In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: MW4PR03CA0073.namprd03.prod.outlook.com (2603:10b6:303:b6::18) To MWHPR1001MB2158.namprd10.prod.outlook.com (2603:10b6:301:2d::17) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: MWHPR1001MB2158:EE_|BN0PR10MB4871:EE_ X-MS-Office365-Filtering-Correlation-Id: 822d9c57-5d53-46b8-f955-08daba498d27 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: QmFjF4heDVxpR4RF0RuIkUymbYdTh3fwgH2hzz7dZ53XN53jnInHvKwLhAwnbhuH653gTMfE2kbwOnJRtXa0uYYWyr5AZFfyoyFdvCD4+gbVTFCdDIOB++LTG6Oj6DXWfRrREdYQ+qfzqtev/iC1nA/+G1BWHkw6+I3Hk03KEj4bKdG/56BWXC/X3PH41fOMV9J1N9UiFfQ62pEvjQLFT1U2xX6y8kAjgHmuvzpiDOSYLGaYqVw10BAwkPadYywNdfGBgXv0e2KS+4PoR9Md4QwKjSYnRscWG+l6F9ohexW8CnAIe6SVRCTM1QB0BfuSxLpf1t60jX5wdNaEJmXZd2c7z9+NZo9bSaPuDmTA2gJBhMSGZcaQ9hlhlWeOadueW/sUcV2i/AO7ENET/OHnJj4i3KCgMiZPAi52bVNUXTwMr3hd+G5KWwITM9j2Z1tbDJhnMCnqcNeufONUzaPTbneiO5Pyiw70i+1LLf1UfL3kpJ2EYEhb8HLAXSCJcRR7T17tIlb84n93nmGNKCgjW2BcN7Mlp7RG1nL/26YSdOq3/Kl71JOMulv2sKgPhABewMsqucMQxugFeQjE7qqNQIEtj/af3BvxCoRfLaIad9u0DHaJfWE74O0RyDioPiP2VX0PBygIpRjCx9bgVguTvrEOOYy9ytrvSRRgyfAX16KeYzL28XxfIHD3DiEZ4HIMlJ+Oihw9hoZq4Yd6dq9HfCpz9kxgP1vCQ1dhhHWj7tS+lMGdwvQfRIJqOR3iqoQopoVBJtk1yhR9d756gJzaicQP6TsOBMf+X8azvgsMQxY= X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:MWHPR1001MB2158.namprd10.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230022)(376002)(39860400002)(396003)(136003)(366004)(346002)(451199015)(36756003)(38100700002)(86362001)(31696002)(2906002)(6486002)(44832011)(478600001)(316002)(4326008)(8676002)(66556008)(66946007)(66476007)(8936002)(5660300002)(6916009)(6506007)(41300700001)(186003)(2616005)(6512007)(53546011)(83380400001)(31686004)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?aW56b003QS9ZWnM5ak9TTDd4N1JNM3U3eDZ2RUl3ZXo0dUkrTmpoWE9vS2x6?= =?utf-8?B?eGsxSGdpL281V1libTUzN1pOUDJDQkJmUms5cUNMQXhhbjZnVThxWXJDK2dV?= =?utf-8?B?UDdjTkRNYzN6Y2R5b2k1aWhtZjFuM2djc0J0T3JuU0NwZ0NQRnE5TkNDRGpx?= =?utf-8?B?anJvSENxKzNCRWQvU25lNFJzR3pkRWd1ekZLUndnUTc1TmtxTWM3WTFYNi9Q?= =?utf-8?B?ZmV0cmdHTlJxODRFdkZZSUZyUnFPN1hxUitGNnBmczJpajVsWG5NRXM2WkQy?= =?utf-8?B?dkoycWxhN2RVK3YrN2RKMWlEVkg2QlJzTHdOdlB4R0U1c1FTYWgyaE1VUUNY?= =?utf-8?B?Q09qVVhRT1FjMEdlWFRLNjlWQllhWDNWR2djTUdRODV1NFRsSTYyYVFWbnJC?= =?utf-8?B?MytqblRYTDQwOThudUthUnU2Qmo5QVExQXF2RmxRSWk3eXdaRGxKV2IwVWdn?= =?utf-8?B?cWVieSs1Nmx0MnVtd2IycjVDaGZrSE5LSzF4eWJLbGVwdUN2bDBlZTJmU0tK?= =?utf-8?B?VGRWMGFFUlFqeHJpWnY3SjB2RHRXV2dsQkFXbWttZmQzb2NkOUNDWE5wTWZt?= =?utf-8?B?ekgxaFl1bHltVDFWcE4yS0JBRURJK2JaVUJBWFNwNkhWZlA3MHc0aHcvdUUy?= =?utf-8?B?Z1lBRDU5Z3REWmRJM3A0ZTJTK21zSW1sUHpOQzBvTG94dWtPN0IzVTY4RWww?= =?utf-8?B?dW1rZGdTOHNpREdBaVEzZjEvcmJBdzhZRk1ZQXlOdTFhS3JjSnkzbHNwUW16?= =?utf-8?B?WXB1SFY4VE9GL1BzOHNMN1NUcmhLRnNjQ0xUV3pVVjZsTzZ0b0RxOTBPOTFz?= =?utf-8?B?d2ZJZ0lnN3pKMi9qaEdTb1lwTDRYaEcrM2hjK0dMak9EcFpxYTNGNnFJWWJQ?= =?utf-8?B?VDdmQ1AwY2oybmtxV20zZ3d3NEt0SWhvSUJwUXVuNGRqRitPVTdFb2p0OWUz?= =?utf-8?B?N2ZQUVlFQjNTdHQ5US8vU0dRRDV3dzV3ZzZOR3NiS2hUWWZUQmd6YnNlZlRS?= =?utf-8?B?dHUyL1A5Qm9sWHQ4MHBuZlE5ZzlzdWNzTWxWRTBMYmEwc1Z3SzRtNHRtb1Bt?= =?utf-8?B?a3ZiNWpnV1JCbW1XUklaeVVEaUpPRVROWDErYjM4Szh5dDdDaXd6RHRUZnkw?= =?utf-8?B?SVprLzlPaW12QVpTczRuYW1QNkYxaXBXaS9JM2VRdDdRQTBUakJ3WThIQlA1?= =?utf-8?B?QWx1TWVMNm54Nmw2dzc2UmpMM2w2OU9NeFlIbU9uQ0FzaUR0MTEzcVp3cjB6?= =?utf-8?B?Sm9lbEpJMDNKWjVlZjBxeS9xVTZ2VUhsREJOU3lEMXFMK0FXSjlVSURtSTNK?= =?utf-8?B?V0ZnNStmVHAzRHoxallHRWJETm1Wb242Yk9vb1RLeDZyYURtcjdrS0c2ZS9s?= =?utf-8?B?N0hLN3RxQ2VBNVRVdTJKSXBoVmRZT0N0d0pueDVQOEJ5bDFKYlN2YzlSWHNB?= =?utf-8?B?MXcrK20xTzJWVmV0OU5FQUxhMXgwYzUxSkwwdS85MTUyN0FWb3puN3pidi8y?= =?utf-8?B?R0s1RjFDM0tiMEQ4clZTeWNCcGJVU3FmdWV2bXdqdlkxeEdhVWtzNWI2RFpy?= =?utf-8?B?K3F2UzdQOGZQZ1kvREpFYWw1NUNhdlZLZDFNc2FoeXhRekFaa2toQm1iMnRi?= =?utf-8?B?ZGtQbFlON2RhSzA2dnRPT08zbDEyN1YrYWY1a0laUzdmSWRwNUUzQTdYYitL?= =?utf-8?B?RUo5SDJwbUJyZm1mdkJ5ZnFtd3hzNkZhS09kVlg5L0hIQW9td3YwT2dWLzc1?= =?utf-8?B?NXJlNm9IS0VtU0hpdDNpeUJpaFd6T3p5cHQ2azZnV2tFYzd0TE1PNi9HVlRm?= =?utf-8?B?M0R4KzdpaG9MZWF5NmpmTjA2UUQ5S2tmMWJZVnJFV0dSZkROMHhRb1FoZXNw?= =?utf-8?B?MUVGNVhTNmcxYmZFR0E4TEJXQjljV0FsZjlpejg5SmoyMHZBK2FHcG1sSVlH?= =?utf-8?B?Vkh0VlJqeDBtdndvQ1poVURtV0pJMWhvNTIzdCtqeEVsMHR3Q0hUNlJXWEli?= =?utf-8?B?UlUxcXRDQ0lYRkpIVHZQZ1VuNmRLSHUyQm9DNk5ETWlrUXNSQ2lSWVFubHNX?= =?utf-8?B?VTNhTER2a1RkeEZFRWdYQ2Q2NVV2SGJsV3pZdERyMjZGR3BsdHBNU0tIaDdj?= =?utf-8?B?QXVBN3RRbDVuVmRGVjBjeTY0ZytLekh2dy9tZThiUE1xQXlSUHRIcDBIdEZ3?= =?utf-8?Q?9V9e+dyatNIVGRS/awUkrY8=3D?= X-OriginatorOrg: oracle.com X-MS-Exchange-CrossTenant-Network-Message-Id: 822d9c57-5d53-46b8-f955-08daba498d27 X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2158.namprd10.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Oct 2022 07:37:09.0870 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: D/u80pASTbGINR9jiLIdx/79iYovTZCBfbmTHtfaCa2I1xAmDD7MwvcEllwJTEAiTEB0jkB3CrftGvVxzQ6rrA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN0PR10MB4871 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1 definitions=2022-10-30_02,2022-10-27_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 spamscore=0 bulkscore=0 suspectscore=0 phishscore=0 malwarescore=0 mlxscore=0 mlxlogscore=999 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2210170000 definitions=main-2210300048 X-Proofpoint-ORIG-GUID: OTIZuWUjGKP036dNimBWO4zXM1myDM6y X-Proofpoint-GUID: OTIZuWUjGKP036dNimBWO4zXM1myDM6y X-Spam-Status: No, score=-7.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi Mike, On 10/27/22 11:07, Mike Frysinger wrote: > On 17 Oct 2022 15:16, Indu Bhagat via Binutils wrote: >> --- /dev/null >> +++ b/libsframe/Makefile.am >> >> +SUBDIRS = testsuite > > since this is new code, can you avoid recurisve makes ? rename the subdir > Makefile.am to local.mk and include them in this Makefile.am instead. > OK. I have made the change in V3 to avoid recursive make. >> +BASEDIR = $(srcdir)/.. > > you don't actually use this anywhere > >> +AM_CPPFLAGS = -D_GNU_SOURCE -I$(srcdir) -I$(srcdir)/../include -I$(srcdir)/../libctf > > you called AC_USE_SYSTEM_EXTENSIONS, so why do you need to use -D_GNU_SOURCE ? > >> +AM_CFLAGS = -std=gnu99 @ac_libsframe_warn_cflags@ @warn@ @c_warn@ @WARN_PEDANTIC@ @WERROR@ > > most other projects have already moved to C11. why do you have to use such > an ancient version ? > Removed -std=gnu99. >> +libsframe_la_CPPFLAGS = -D_GNU_SOURCE -I$(srcdir) -I$(srcdir)/../include \ >> + -I$(srcdir)/../libctf > > you should be using $(AM_CPPFLAGS) instead of manually duplicating the value > >> --- /dev/null >> +++ b/libsframe/configure.ac >> >> +AC_CONFIG_MACRO_DIR(..) >> +AC_CONFIG_MACRO_DIR(../config) >> +AC_CONFIG_MACRO_DIR(../bfd) > > you're setting ACLOCAL_AMFLAGS already. pretty sure you don't need these. > >> +MISSING=`cd $ac_aux_dir && ${PWDCMD-pwd}`/missing >> +AC_CHECK_PROGS([ACLOCAL], [aclocal], [$MISSING aclocal]) >> +AC_CHECK_PROGS([AUTOCONF], [autoconf], [$MISSING autoconf]) >> +AC_CHECK_PROGS([AUTOHEADER], [autoheader], [$MISSING autoheader]) > > why do you need this logic ? isn't this covered by AM_MAINTAINER_MODE ? > >> +# Figure out what compiler warnings we can enable. >> +# See config/warnings.m4 for details. > > use `dnl` for comments, not `#` > >> +ACX_PROG_CC_WARNING_OPTS([-W -Wall -Wno-narrowing -Wwrite-strings \ >> + -Wmissing-format-attribute], [warn]) >> +ACX_PROG_CC_WARNING_OPTS([-Wstrict-prototypes -Wmissing-prototypes \ >> + -Wold-style-definition], [c_warn]) >> +ACX_PROG_CC_WARNING_ALMOST_PEDANTIC([-Wno-long-long]) >> + >> +# Only enable with --enable-werror-always until existing warnings are >> +# corrected. >> +ACX_PROG_CC_WARNINGS_ARE_ERRORS([manual]) > > this project is new. why aren't the warnings fixed to begin with ? > >> +DEJAGNU_CHECK_VERSION >> +AM_CONDITIONAL([HAVE_COMPAT_DEJAGNU], [test "x$ac_cv_dejagnu_compat" = "xyes"]) >> + >> +COMPAT_DEJAGNU=$ac_cv_dejagnu_compat >> +AC_SUBST(COMPAT_DEJAGNU) >> + >> +AM_MAINTAINER_MODE >> +AM_INSTALL_LIBBFD >> +ACX_PROG_CC_WARNING_OPTS([-Wall], [ac_libsframe_warn_cflags]) > > you already probed -Wall above. do you really need to do it again ? > I have sanitized the bits around warning flags, and the code should be warning-free now. Also addressed the other comments above. >> +AC_FUNC_MMAP >> +AC_CHECK_HEADERS(byteswap.h endian.h) >> + >> +dnl Check for bswap_{16,32,64} >> +AC_CHECK_DECLS([bswap_16, bswap_32, bswap_64], [], [], [[#include ]]) >> +AC_CHECK_DECLS([asprintf, vasprintf, stpcpy]) > > you aren't using most of this. are you just copying & pasting from another > tree ? > >> +GNU_MAKE_JOBSERVER > > do you actually need this ? > I saw the commit d546b61084cec687e0063b2e0e169b4690341c23 and judged that this may be needed. >> --- /dev/null >> +++ b/libsframe/testsuite/libsframe.decode/Makefile.am >> @@ -0,0 +1,17 @@ >> +if HAVE_COMPAT_DEJAGNU >> + check_PROGRAMS = be-flipping frecnt-1 frecnt-2 >> +else >> + check_PROGRAMS = >> +endif > > this style complicates things. use: > > check_PROGRAMS = > if HAVE_COMPAT_DEJAGNU > check_PROGRAMS += ... > endif > -mike OK, changed to the suggested style. Thanks for reviewing. I have addressed the comments in V3.