Nut upsdev - Apr 2006 - Re: [nut-commits] svn commit r414 - in trunk: . drivers

> Author: nba-guest
> Date: Mon Apr 24 20:45:39 2006
> New Revision: 414
>
> Modified:
>    trunk/CHANGES
>    trunk/drivers/cpsups.c
> Log:
> Fix C++ style declarations between statements
>
> Modified: trunk/CHANGES
Niels,

What is the reason for 'fixing' this? Constructions like these are used
in
many locations throughout the sources (not only in the drivers, but also
in common and server). If there is a compelling reason to change this, we
need to do this in other parts as well. That will be quite an effort I
presume, so we should really consider if this is absolutely needed.
Personally, I don't think it is a big issue, since we would have seen many
more complaints about this.

Arjen
 =============================================================================>
--- trunk/CHANGES	(original)
> +++ trunk/CHANGES	Mon Apr 24 20:45:39 2006
> @@ -1,3 +1,7 @@
> +Mon Apr 24 20:45:37 UTC 2006 / Niels Baggesen
<nba@users.sourceforge.net>
> +
> + - drivers/cpsups.c: Fix C++ style declarations between statements
> +
>  Mon Apr 24 20:44:51 UTC 2006 / Niels Baggesen
<nba@users.sourceforge.net>
>
>   - man/upscode2.8: Fix speling in man page
>
> Modified: trunk/drivers/cpsups.c
>
=============================================================================>
--- trunk/drivers/cpsups.c	(original)
> +++ trunk/drivers/cpsups.c	Mon Apr 24 20:45:39 2006
> @@ -147,6 +147,7 @@
>  	char values[20][200], *pos;
>  	int i = 0, battremain, length, rseconds;
>  	double rminutes;
> +	char temp1, *tmp1;
>
>  	while ((pollstatusmap[i].end != 0))
>  	{
> @@ -159,8 +160,8 @@
>  /*	These are used to hold status of UPS.
>   *	val1 = online/onbattery status
>   */
> -	char temp1=values[6][0];
> -	char *tmp1=&temp1;
> +	temp1=values[6][0];
> +	tmp1=&temp1;
>
>  	if ((*tmp1 & CPS_STAT_OL) && !(*tmp1 & CPS_STAT_OB))
>  		status_set("OL");
>
> _______________________________________________
> nut-commits mailing list
> nut-commits@lists.alioth.debian.org
> http://lists.alioth.debian.org/mailman/listinfo/nut-commits
>
>

>> > Fix C++ style declarations between statements
>> What is the reason for 'fixing' this? Constructions like these
are used
>> in
>> many locations throughout the sources (not only in the drivers, but
also
>> in common and server). If there is a compelling reason to change this,
>> we
>> need to do this in other parts as well. That will be quite an effort I
>> presume, so we should really consider if this is absolutely needed.
>> Personally, I don't think it is a big issue, since we would have
seen
>> many
>> more complaints about this.
> There are no other instances like this.
Yes there are, quite a few actually. Take for instance
'drivers/mge-shut.c', in the function 'char_read' where the
variable 'now'
is defined:

	if (FD_ISSET (upsfd, &readfs)) {
		int now = read (upsfd, bytes, size - readen);

This is essentially the same construction. Even in 'server/sstate.c' in
function 'sstate_connect' we have things like this (look in the Stable
tree, since I removed this in trunk before, because I didn't like the
clumsy construction):

	if (ret < 0) {
		time_t	now;
> Putting declarations between statements is not valid C.
That depends on which version you're speaking. In C99 this is allowed and
it seems that many drivers use them too (have a look and be just as
surprized as I was when I first came across this).
> There is a while block preceding the bad spot.
I know older compilers may choke on this, yet I wonder why it hasn't been
a problem in other locations in the tree so far. Your compiler should
choke on all these too.

Arjen

On Tue, Apr 25, 2006 at 08:51:40AM +0200, Arjen de Korte
wrote:
> > Log:
> > Fix C++ style declarations between statements
> 
> What is the reason for 'fixing' this? Constructions like these are
used in
> many locations throughout the sources (not only in the drivers, but also
> in common and server). If there is a compelling reason to change this, we
> need to do this in other parts as well. That will be quite an effort I
> presume, so we should really consider if this is absolutely needed.
> Personally, I don't think it is a big issue, since we would have seen
many
> more complaints about this.
There are no other instances like this.

Putting declarations between statements is not valid C.
There is a while block preceding the bad spot.

Changing it back as it was before, I get

hera:drivers> make
gcc -g -I../include -O -Wall -Wsign-compare  -c  cpsups.c
cpsups.c: In function `scan_poll_values':
cpsups.c:162: parse error before `char'
cpsups.c:165: `tmp1' undeclared (first use in this function)
cpsups.c:165: (Each undeclared identifier is reported only once
cpsups.c:165: for each function it appears in.)
*** Error code 1
make: Fatal error: Command failed for target `cpsups.o'
hera:drivers> gcc -v
Reading specs from
/usr/local/lib/gcc-lib/sparc-sun-solaris2.8/2.95.3/specs
gcc version 2.95.3 20010315 (release)

/Niels

-- 
Niels Baggesen -- @home -- ?rhus -- Denmark -- niels@baggesen.net
The purpose of computing is insight, not numbers  --  R W Hamming