Bug 110826 - bad source code
bad source code
Status: CLOSED WONTFIX
Product: Fedora
Classification: Fedora
Component: grub (Show other bugs)
1
All Linux
medium Severity medium
: ---
: ---
Assigned To: Peter Jones
Mike McLean
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2003-11-24 12:12 EST by d.binderman
Modified: 2007-11-30 17:10 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2005-01-11 11:35:22 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


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

  None (edit)
Description d.binderman 2003-11-24 12:12:51 EST
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 17:20:19 EST
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 07:09:57 EST
>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 10:17:36 EST
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-02 23:09:29 EST
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 07:21:41 EST
Eli: That is completely unrelated. You need ncurses-devel.
Comment 6 Peter Jones 2005-01-11 11:35:22 EST
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.