Coding Horrors: You can gather all your technical debt into one easy framework!

The Something Awful Forums > Discussion > Serious Hardware/Software Crap > The Cavern of COBOL > Coding Horrors: You can gather all your technical debt into one easy framework!

«‹›1503 »

pseudorandom name: May 6, 2007

Since the option is called -Wextra and not -Weverything, it is clear that none of you actually care about the warnings omitted from -Wall and included in -Wextra.

# ? Feb 23, 2014 23:25

Adbot: ADBOT LOVES YOU

# ? May 14, 2024 21:35

Suspicious Dish: Sep 24, 2011; 2020 is the year of linux on the desktop, bro; Fun Shoe

vOv posted:

You can't spell -funroll-loops without 'fun'!

I, too, use gentoo

# ? Feb 23, 2014 23:36

evensevenone: May 12, 2001; Glass is a solid.

pseudorandom name posted:

Since the option is called -Wextra and not -Weverything, it is clear that none of you actually care about the warnings omitted from -Wall and included in -Wextra.

-Weverything is in clang.

# ? Feb 24, 2014 00:23

shrughes: Oct 11, 2008; (call/cc call/cc)

In Windows it's "My Documents" but in Unix it's "You Context".

# ? Feb 24, 2014 00:29

Suspicious Dish: Sep 24, 2011; 2020 is the year of linux on the desktop, bro; Fun Shoe

shrughes posted:

In Windows it's "My Documents" but in Unix it's "You Context".

Excuse me?

# ? Feb 24, 2014 00:44

Hughlander: May 11, 2005

Otto Skorzeny posted:

No, filing bugs will accomplish nothing here. It's by design (read: rms diktat) and implied in the name that the -pedantic switch does not work right, cf. the manual:

e: expanded quote slightly

Embrace and Extend indeed.

# ? Feb 24, 2014 02:16

Dren: Jan 5, 2001; Pillbug

goto has legitimate uses for error handling in C, please put down your torches. And it's not that bad to use it in C++ instead of exceptions.

# ? Feb 24, 2014 02:34

Suspicious Dish: Sep 24, 2011; 2020 is the year of linux on the desktop, bro; Fun Shoe

Dren posted:

goto has legitimate uses for error handling in C, please put down your torches. And it's not that bad to use it in C++ instead of exceptions.

Nobody was arguing goto was terrible, Dren.

# ? Feb 24, 2014 02:45

Deus Rex: Mar 5, 2005

Dren posted:

goto has legitimate uses for error handling in C, please put down your torches. And it's not that bad to use it in C++ instead of exceptions.

How's that? In this case the gotos were used to avoid repeating code that freed resources before returning the value of err (which was assigned before the fail label). If it were in C++ you'd just return err and RAII would take care of the rest.

# ? Feb 24, 2014 02:53

FlapYoJacks: Feb 12, 2009

fritz posted:

goto should no longer be considered harmful in 2014 when we have so many more control structures available to us now, such as "while".

longjmp

# ? Feb 24, 2014 02:57

Dren: Jan 5, 2001; Pillbug

Suspicious Dish posted:

Nobody was arguing goto was terrible, Dren.

I kinda felt like Westie was when Westie called goto a dinosaur.

Deus Rex posted:

How's that? In this case the gotos were used to avoid repeating code that freed resources before returning the value of err (which was assigned before the fail label). If it were in C++ you'd just return err and RAII would take care of the rest.

The error is the repeated line of code, not the goto. If the code had been designed so that the resources are tied to an object and freed when the object dies the error still could have easily occurred. It would have looked like this:

C++ code:

if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
    return err;
    return err;

or, if exceptions were used:

C++ code:

if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
    throw SSLException(err);
    throw SSLException(err);

# ? Feb 24, 2014 03:15

Deus Rex: Mar 5, 2005

I just mean that I don't see why one would use gotos for error handling in C++. You can just return an error code where the goto call would have been. The only purpose goto serves in C error handling code, at least to my understanding, is to keep all of the resource cleanup code in one place (at the end of the function), which you don't need with RAII.

# ? Feb 24, 2014 03:28

Dren: Jan 5, 2001; Pillbug

Deus Rex posted:

I just mean that I don't see why one would use gotos for error handling in C++. You can just return an error code where the goto call would have been. The only purpose goto serves in C error handling code, at least to my understanding, is to keep all of the resource cleanup code in one place (at the end of the function), which you don't need with RAII.

I see what you mean. I can't think of a reason besides integrating C library and being lazy. I wouldn't flag it in a code review if it was a one off.

# ? Feb 24, 2014 03:46

raminasi: Jan 25, 2005; a last drink with no ice

Deus Rex posted:

I just mean that I don't see why one would use gotos for error handling in C++. You can just return an error code where the goto call would have been. The only purpose goto serves in C error handling code, at least to my understanding, is to keep all of the resource cleanup code in one place (at the end of the function), which you don't need with RAII.

C++ code:

int do_something() {
    int err;
    if (err = do_stage_1()) { goto cleanup_1; }
    if (err = do_stage_2()) { goto cleanup_2; }
    if (err = do_stage_3()) { goto cleanup_3; }
    err = do_thing();
cleanup_3:
    do_cleanup_3();
cleanup_2:
    do_cleanup_2();
cleanup_1:
    do_cleanup_1();
    return err;
}

# ? Feb 24, 2014 04:26

HORATIO HORNBLOWER: Sep 21, 2002; no ambition,
no talent,
no chance

Dren posted:

It would have looked like this:

C++ code:

if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
    return err;
    return err;

or, if exceptions were used:

C++ code:

if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
    throw SSLException(err);
    throw SSLException(err);

This is an interesting contrast since a thrown exception is unlikely to be treated as a success condition, whereas "error code equals 0" is a common idiom in C/C++.

# ? Feb 24, 2014 04:35

Plorkyeran: Mar 22, 2007; To Escape The Shackles Of The Old Forums, We Must Reject The Tribal Negativity He Endorsed

That is not at all exception-safe, and is more error prone (and sometimes even more code) than just using the C++ constructs that are exception-safe. goto for error handling is totally valid in C, but not so much in C++.

# ? Feb 24, 2014 04:35

Deus Rex: Mar 5, 2005

GrumpyDoctor posted:

C++ code:

int do_something() {
    int err;
    if (err = do_stage_1()) { goto cleanup_1; }
    if (err = do_stage_2()) { goto cleanup_2; }
    if (err = do_stage_3()) { goto cleanup_3; }
    err = do_thing();
cleanup_3:
    do_cleanup_3();
cleanup_2:
    do_cleanup_2();
cleanup_1:
    do_cleanup_1();
    return err;
}

Well in this case presumably do_stage_n() each allocate some object and stick it in some global variable, so RAII doesn't help you either way you do it (in the case of using exceptions, what happens when an exception is thrown? now the caller has to free those resources I think, or you use that same goto fail idiom but throw an exception instead of returning). I would imagine something like this:

C++ code:

int do_something() {
    int err;
    foo a;
    if ((err = do_stage_1(a))) return err;
    foo b;
    if ((err = do_stage_2(b))) return err;
    foo c;
    if ((err = do_stage_3(c))) return err;

    err = do_thing(a, b, c);
    return err;
}

where do_stage_1/2/3 accept foo by reference and do whatever mutations they need to would be preferable and easier to reason about.

edit: i'd like to add that i hardly know C++ and don't write it for real ever, so if this is a horror itself at least know it's not deployed anywhere

Deus Rex fucked around with this message at 06:15 on Feb 24, 2014

# ? Feb 24, 2014 05:56

Plorkyeran: Mar 22, 2007; To Escape The Shackles Of The Old Forums, We Must Reject The Tribal Negativity He Endorsed

If you can't do anything about the fact that the steps are just mutating global state, the most direct translation to something exception safe is:

C++ code:

template<typename Func>
struct scope_guard {
    Func f;
    ~scope_guard() { f(); }
}

template<typename Func>
scope_guard<Func> make_scope_guard(Func f) {
    return scope_guard<Func>{f};
}

int do_something() {
    auto s1 = make_scope_guard(do_cleanup_1);
    if (int err = do_stage_1()) { return err; }

    auto s2 = make_scope_guard(do_cleanup_2);
    if (int err = do_stage_2()) { return err; }

    auto s3 = make_scope_guard(do_cleanup_3);
    if (int err = do_stage_3()) { return err; }

    return do_thing();
}

# ? Feb 24, 2014 06:23

Dren: Jan 5, 2001; Pillbug

HORATIO HORNBLOWER posted:

This is an interesting contrast since a thrown exception is unlikely to be treated as a success condition, whereas "error code equals 0" is a common idiom in C/C++.

For me it's easy to imagine an exception slotting exactly into where the goto is such that the catch block does the exact same cleanup as the goto and returns err.

# ? Feb 24, 2014 07:43

pseudorandom name: May 6, 2007

You could, but it'd perform terribly.

# ? Feb 24, 2014 08:20

b0lt: Apr 29, 2005

pseudorandom name posted:

You could, but it'd perform terribly.

Exceptions are generally free in the non-exceptional case, and I don't think any implementation of SSL is particularly concerned about maximizing connections per second to hosts that are forging their identity.

edit: Also hypothetically a sufficiently smart compiler could optimize a throw with a visible catch into a jump.

b0lt fucked around with this message at 12:12 on Feb 24, 2014

# ? Feb 24, 2014 12:02

pseudorandom name: May 6, 2007

Oh, for some reason I read Dren's suggestion as the thrown exception being the normal case. Nevermind then.

# ? Feb 24, 2014 12:14

Deus Rex: Mar 5, 2005

b0lt posted:

sufficiently smart compiler

# ? Feb 24, 2014 15:55

Cadoc: Mar 5, 2007

Recently I was forced to change the line $_SERVER['REMOTE_ADDR'] = IP_FROM_PAYMENT_SERVER_PROVIDER to 1=1 for the payment successfully received function in an multi million online shop. Reason was the fact that the new webserver hoster does not want to set ANY server variable and has the webserver behind a proxy. Apperently "beeing audited" is better than possibly beeing sued for aiding fraud.

# ? Feb 24, 2014 16:10

biznatchio: Mar 31, 2001; Buglord

Cadoc posted:

Recently I was forced to change the line $_SERVER['REMOTE_ADDR'] = IP_FROM_PAYMENT_SERVER_PROVIDER to 1=1 for the payment successfully received function in an multi million online shop. Reason was the fact that the new webserver hoster does not want to set ANY server variable and has the webserver behind a proxy. Apperently "beeing audited" is better than possibly beeing sued for aiding fraud.

Make sure you've gotten your objections to the change and what the potential security ramifications of it are in writing. Chances are it'll never come up, but if it does and they go looking for a scapegoat, you'll want to be sure you've covered your rear end.

# ? Feb 24, 2014 17:04

necrotic: Aug 2, 2005; I owe my brother big time for this!

biznatchio posted:

Make sure you've gotten your objections to the change and what the potential security ramifications of it are in writing. Chances are it'll never come up, but if it does and they go looking for a scapegoat, you'll want to be sure you've covered your rear end.

This. Use the commit message to detail your issues with the change in addition to emails / issue comments around the change.

# ? Feb 24, 2014 20:34

shodanjr_gr: Nov 20, 2007

Dren posted:

The error is the repeated line of code, not the goto. If the code had been designed so that the resources are tied to an object and freed when the object dies the error still could have easily occurred. It would have looked like this:

It's probably been pointed out already, but the library in question was written in C...

# ? Feb 25, 2014 00:54

apseudonym: Feb 25, 2011

shodanjr_gr posted:

It's probably been pointed out already, but the library in question was written in C...

The point was that lots of people have been running around shouting about gotos being harmful, ignoring the fact that if we had been in C++ with RAII the code would be

code:


if(...)
 return err;
 return err;

Gotos for error handling are the right thing to do in C. Some people have a knee jerk 'gotos considered harmful' reaction even though this is not what Dijkstra was complaining about.

# ? Feb 25, 2014 01:04

Jabor: Jul 16, 2010; #1 Loser at SpaceChem

Automated merges considered harmful.

# ? Feb 25, 2014 01:06

apseudonym: Feb 25, 2011

Jabor posted:

Automated merges considered harmful.

Lack of code review considered harmful.

# ? Feb 25, 2014 01:09

carry on then: Jul 10, 2010; by VideoGames
(and can't post for 10 years!)

Programmers other than me considered harmful. :v:

# ? Feb 25, 2014 01:30

Dren: Jan 5, 2001; Pillbug

apseudonym posted:

Lack of code review considered harmful.

Peer review doesn't help when your peers are as bad as you.

# ? Feb 25, 2014 01:32

shodanjr_gr: Nov 20, 2007

apseudonym posted:

The point was that lots of people have been running around shouting about gotos being harmful, ignoring the fact that if we had been in C++ with RAII the code would be
code:
if(...)
 return err;
 return err;
Gotos for error handling are the right thing to do in C. Some people have a knee jerk 'gotos considered harmful' reaction even though this is not what Dijkstra was complaining about.

I completely agree, there's a reason why goto is one of the standard patterns for doing cleanup in the linux kernel. The way some of the comments are worded however imply that Apple should have (re)written everything in C++, as if that's a viable thing to do for a years-old cryptography library that probably hooks up at a bunch of different places in their iOS/OSX stacks...

# ? Feb 25, 2014 02:07

HORATIO HORNBLOWER: Sep 21, 2002; no ambition,
no talent,
no chance

shodanjr_gr posted:

I completely agree, there's a reason why goto is one of the standard patterns for doing cleanup in the linux kernel. The way some of the comments are worded however imply that Apple should have (re)written everything in C++, as if that's a viable thing to do for a years-old cryptography library that probably hooks up at a bunch of different places in their iOS/OSX stacks...

Hmm, maybe it's NOT a great idea to have a critical library be difficult to maintain and written in a crufty old language that doesn't support any modern control structures for error recovery? But no, no, can't rewrite the thing, because as we all know, rewriting software runs the risk of introducing horrible bugs like, say, accidentally skipping verification of SSL certificates....

# ? Feb 25, 2014 02:14

apseudonym: Feb 25, 2011

HORATIO HORNBLOWER posted:

Hmm, maybe it's NOT a great idea to have a critical library be difficult to maintain and written in a crufty old language that doesn't support any modern control structures for error recovery? But no, no, can't rewrite the thing, because as we all know, rewriting software runs the risk of introducing horrible bugs like, say, accidentally skipping verification of SSL certificates....

I agree. Let's rewrite it in Haskell.

# ? Feb 25, 2014 02:19

shrughes: Oct 11, 2008; (call/cc call/cc)

HORATIO HORNBLOWER posted:

Hmm, maybe it's NOT a great idea to have a critical library be difficult to maintain and written in a crufty old language that doesn't support any modern control structures for error recovery? But no, no, can't rewrite the thing, because as we all know, rewriting software runs the risk of introducing horrible bugs like, say, accidentally skipping verification of SSL certificates....

Look at good security people talking about security stuff, and they seem to always post, whenever the discussion comes up, that C code is generally much more auditable than C++ code.

The problem here was more about not using braces, probably because the people at Apple are line count spergs or something.

# ? Feb 25, 2014 02:36

Steve French: Sep 8, 2003

Dren posted:

Peer review doesn't help when your peers are as bad as you.

If you actually believe this, then the bad peer is you.

# ? Feb 25, 2014 03:01

gonadic io: Feb 16, 2011; >>=

apseudonym posted:

I agree. Let's rewrite it in Haskell.

not enough verification, I vote for Agda.

# ? Feb 25, 2014 03:02

Suspicious Dish: Sep 24, 2011; 2020 is the year of linux on the desktop, bro; Fun Shoe

Dren posted:

Peer review doesn't help when your peers are as bad as you.

I hope I never have to work with you.

# ? Feb 25, 2014 03:09

Adbot: ADBOT LOVES YOU

# ? May 14, 2024 21:35

Coffee Mugshot: Jun 26, 2010; by Lowtax

apseudonym posted:

Lack of code review considered harmful.

Honestly, I doubt that given a code review for this, which was probably one of many files, you would have caught the error on a first pass. Especially since the code is so logically simple. It's like when someone writes the the word "the" twice and your eyes kind of just combine them.

# ? Feb 25, 2014 03:12

The Something Awful Forums > Discussion > Serious Hardware/Software Crap > The Cavern of COBOL > Coding Horrors: You can gather all your technical debt into one easy framework!

«‹›1503 »