by

Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Activity
    cecio
    @cecio
    Here we are
    Carlo Mandelli
    @camandel
    SYNauth.c:532:1: warning: label ‘exit_success’ defined but not used [-Wunused-label]
    compiling SYNwall/SYNgate I get warnings about unused labels. I can ignore them with "-Wno-unused-label" but is there a reason to keep unused labels into the code? Thanks
    cecio
    @cecio
    yes, there is (or better, there was) but may be I changed my mind on that: the reason to keep them was to have the source structure ready to "implement" new/different features/behavior, at least during the development phase. But actually, right now may be it's better to have a clean compilation output, to spot out possible new warnings in an easier way....so remove them may be it's a good idea
    Carlo Mandelli
    @camandel
    Opened issue SYNwall/SYNwall#16 to keep track of that
    Carlo Mandelli
    @camandel
    The general use of "goto" statement into kernel modules code in place of "if/then/else" is for performance or just to keep the code readable avoiding huge nasted "if/then"? Once compiled both of them become JMP instructions of is there a difference? Thanks
    cecio
    @cecio
    mainly is for readability of code: going without "goto", as you already pointed out, means to have a mess in terms of if/then/else constructs. Let's say that, if used for specific reasons, the "goto" is not to ban at all in code. In this case, for example as an exit condition of a function, it is not only acceptable, but even suggested. In terms of performance it probably does not change a lot. And yes, you are right: once compiled, even the nested if/then/else are most probably "resolved" with jumps statement by the optimizer
    cecio
    @cecio
    quite interesting: on the linux kernel folder, the command grep -r "goto " | wc -l returns something like 149417 :-)
    cecio
    @cecio

    With gcc 9.3:

       if ( a < 100) {
          if ( a < 90 ){
              if ( a < 80 ) {
                  return 1;
              }
          }
       }
       return 0;

    the following is produced

            push    rbp
            mov     rbp, rsp
            mov     DWORD PTR [rbp-4], edi
            cmp     DWORD PTR [rbp-4], 99
            jg      .L2
            cmp     DWORD PTR [rbp-4], 89
            jg      .L2
            cmp     DWORD PTR [rbp-4], 79
            jg      .L2
            mov     eax, 1
            jmp     .L3
    .L2:
            mov     eax, 0
    .L3:
            pop     rbp
            ret

    As you can see tge jg just replicate a goto-like behavior

    cecio
    @cecio
    @camandel if you want, I can assign the Issue #16 to you...thanks
    Carlo Mandelli
    @camandel
    Yes, and thank you for the detailed explaination about "goto"
    Carlo Mandelli
    @camandel
    About "pragma" directives: it seems "diagnostic pragmas" have been introduced in gcc 4.2.4 (May 2008).
    I managed to ignore warnings at function level (#pragma GCC diagnostic ignored "-Wunused-label") with gcc 4.4.7 but I don't have lab systems to easily install older gcc versions.
    cecio
    @cecio
    it sounds great! Thank you! I think we can use this at this point, since the implementation is old enough. In this way we can just ignore the warning in the specific portion of codes. Just as a reminder, we can add a comment there saying that in case of older GCC versions, the directives can just be removed (if they are causing errors)
    Carlo Mandelli
    @camandel
    UDP ports 123 and 53 are hard-coded into "check_udp_blacklist" function. A new parameter to overwrite the default should not be so useful but I think this behavior should be reported into README file (I needed to query an internal dns server on the same SYNwall mesh network)
    cecio
    @cecio
    yes, that was definitely a kludge I did. You are rising a good point (internal SYNwall newtwork request)...
    for sure in the README it should be documented. I'm thinking about the parameter...
    cecio
    @cecio
    For the time being, I added a comment in README...thanks for the note!