Bug 110826 - bad source code
Summary: bad source code
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: grub
Version: 1
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Peter Jones
QA Contact: Mike McLean
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2003-11-24 17:12 UTC by d.binderman
Modified: 2007-11-30 22:10 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2005-01-11 16:35:22 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Output of rpm --rebuild of grub-0.93-7.src.rpm (91.38 KB, text/plain)
2004-03-03 04:09 UTC, Eli
no flags Details

Description d.binderman 2003-11-24 17:12:51 UTC
Description of problem:

I just tried to compile package grub-0_93-6 from Fedora.

The compiler said

1.

fsys_reiserfs.c:761: warning: operation on `depth' may be undefined

The source code is

          cache = read_tree_node (INFO->blocks[depth], --depth);

Deeply unwise code to modify depth in the same statement as 
reading from it. Suggest new code

	cache = read_tree_node (INFO->blocks[depth], depth - 1);
	--depth;

I seem to have reported this bug back in Redhat 8.0, and it still
doesn't seem to be fixed yet.

2.

builtins.c(145): error: expected a ";"
      {

The source code is

/* blocklist */
static int
blocklist_func (char *arg, int flags)
{
  char *dummy = (char *) RAW_ADDR (0x100000);
  int start_sector;
  int num_sectors = 0;
  int num_entries = 0;
  int last_length = 0;

  /* Collect contiguous blocks into one entry as many as possible,
     and print the blocklist notation on the screen.  */
  static void disk_read_blocklist_func (int sector, int offset, int 
length)
    {
      if (num_sectors > 0)

Nested functions are not supported in ISO C - suggest move function
disk_read_blocklist_func out of function blocklist_func.


Version-Release number of selected component (if applicable):
grub-0_93-6 

How reproducible:


Steps to Reproduce:
1.
2.
3.
  
Actual results:


Expected results:


Additional info:

Comment 1 Jeremy Katz 2003-11-24 22:20:19 UTC
This is completely legal.  Nested functions are perfectly valid C99
and --depth is also completely fine in this context -- using (depth -
1) instead provides different semantics.

Comment 2 d.binderman 2003-11-25 12:09:57 UTC
>This is completely legal.

Use of nested functions is a GNU C only extension.

See http://gcc.gnu.org/onlinedocs/gcc/C-Extensions.html#C%20Extensions
for details.

I had hoped you would be interested in making the code
work on a larger range of compilers than just GNU C.

>Nested functions are perfectly valid C99

Not on any C99 compiler I tried.

>--depth is also completely fine in this context 

Again not true. Try reading K&R 2, section 2.12, page 53.





Comment 3 Miloslav Trmac 2003-11-25 15:17:36 UTC
read_tree_node (E(depth), --depth) is not legal because
there is no sequence point between use of depth in E and its modification.
And no, nested functions are not in ISO/IEC 9989-1999.

Comment 4 Eli 2004-03-03 04:09:29 UTC
Created attachment 98231 [details]
Output of rpm --rebuild of grub-0.93-7.src.rpm

Adding insult to injury, grub won't even rebuild on a Fedora Core 1 system
using rpmbuild --rebuild.  Output from that failed build is attached.

Comment 5 Miloslav Trmac 2004-03-23 12:21:41 UTC
Eli: That is completely unrelated. You need ncurses-devel.

Comment 6 Peter Jones 2005-01-11 16:35:22 UTC
Right now, both of these issues compile cleanly and apparently work as expected
with gcc.  While in principle it's true that grub should be as portable as
possible, GCC on Linux is the supported compiler for Fedora and RHEL.

This issue needs to be addressed upstream.


Note You need to log in before you can comment on or make changes to this bug.