Skip to content

Commit

Permalink
xdu.c: Fixes need to compile xdu.c with -O2
Browse files Browse the repository at this point in the history 
I've choosen 'n/a' as initial string, because this clearly isn't
a filename.
  • Loading branch information
@thomas
thomas committed Jan 25, 2019
1 parent c8f382c commit b5fda28
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions xdu.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -999,7 +999,7 @@ void savepschildren(FILE* fp, struct node* nodep, struct rect rect, int showsize
height,
top;
struct node* np;
char *name,
char *name="n/a",
label[1024];
struct rect subrect;

Expand Down Expand Up @@ -1148,7 +1148,7 @@ void fprintsvgnode(FILE* fp, struct rect rect)
void fprintsvgnodetext(FILE* fp, int x, int y, long long size, char* name, int showsize)
{
char buffer[1024],
*text;
*text="n/a";

switch (showsize) {
case 0:
Expand Down

3 comments on commit b5fda28

@donald
Copy link
Contributor

@donald donald commented on b5fda28 Feb 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler detects the possible use of the uninitialized pointers only with -O2 ? Interesting.

@pmenzel
Copy link
Contributor

@pmenzel pmenzel commented on b5fda28 Feb 7, 2019 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@donald
Copy link
Contributor

@donald donald commented on b5fda28 Feb 8, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same with latest gcc. See https://godbolt.org/z/yt-uN_

Well, -O0 is "Reduce compilation time and make debugging produce the expected results."

With -O1 it does a data flow analysis (producing the warning by the way) and tries to avoid conditional jumps.

Fun fact 1: It does so by preloading one of the case options into the variable. So in the default case we will no longer see random stack value in it, but the value of case 1 instead. I guess its legit to replace a "undefined" value with any other value.

Fun fact 2 : It tries to avoid a jump by using "cmove" (conditional move). However, as the conditional distinguished between case 2 and default and in the default case, the value is undefined anyway, it could have avoided the conditional move as well and just load the case 2 value instead.

In this reduced case (with no side effects in the case branches) it could even be reduced to

cmpl    $2, %eax         // i==2 ?
movl    $.LC0, %edi      // test="a"
movl    $.LC1, %eax     // tmp="b"
cmove   %rax, %rdi      // test=tmp if i==2
call out();

and avoid any conditional jump and possible pipeline stall.

Oh, I just see, that clang does exactly this! Switch to x64-46 clang (trunk).

A pitty, the kernel can't be compiler with clang yet (or can it?)

Please sign in to comment.