Stories from BIND9 refactoring Dealing with code that can drink - - PowerPoint PPT Presentation

▶
stories from bind9 refactoring
SMART_READER_LITE
LIVE PREVIEW

Stories from BIND9 refactoring Dealing with code that can drink - - PowerPoint PPT Presentation

Sep 07, 2023 564 likes •1.78k views

Stories from BIND9 refactoring Dealing with code that can drink legally Witold Krcicki February 3, 2019 Witold Krcicki Stories from BIND9 refactoring February 3, 2019 1 / 23 What is old code? Witold Krcicki Stories from BIND9

slide-1
SLIDE 1

Stories from BIND9 refactoring

Dealing with code that can drink legally Witold Kręcicki February 3, 2019

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 1 / 23

slide-2
SLIDE 2

What is old code?

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 2 / 23

slide-3
SLIDE 3

What is old code?

Software developer in mid 90’s in Poland

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 2 / 23

slide-4
SLIDE 4

What is old code?

Software developer in mid 90’s in Poland Write a simple ’phonebook’ application

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 2 / 23

slide-5
SLIDE 5

What is old code?

Software developer in mid 90’s in Poland Write a simple ’phonebook’ application

56 6552355

Area code Subscriber number

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 2 / 23

slide-6
SLIDE 6

What is old code?

Software developer in mid 90’s in Poland Write a simple ’phonebook’ application

56 6552355

Area code Subscriber number That’s easy!

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 2 / 23

slide-7
SLIDE 7

const char areas[][16] = {"","","","","","","","","","","","","Krakow",""... const char* phonearea(const char* num) { int i, area; if (strlen(num) != 9) { return ""; } for (i = 0; i < 9; i++) { if (num[i]<’0’ && num[i]>’9’) { return ""; } } area = 10*(num[0]-’0’) + num[1]-’0’; return areas[area]; }

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 3 / 23

slide-8
SLIDE 8

const char areas[][16] = {"","","","","","","","","","","","","Krakow",""....}; const char* phonearea(const char* num) { int i, area; if (strlen(num) != 9) { return ""; } for (i = 0; i < 9; i++) { if (num[i]<’0’ && num[i]>’9’) { return ""; } } if (!strncmp(num, "601", 3)) { return "M-ERA"; } if (!strncmp(num, "602", 3)) { return "M-PLUS"; } if (!strncmp(num, "501", 3)) { return "M-IDEA"; } area = 10*(num[0]-’0’) + num[1]-’0’; return areas[area]; } Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 4 / 23

slide-9
SLIDE 9

const char areas[][16] = {"","","","","","","","","","","","","Krakow",""....}; const char* phonearea(const char* num) { int i, area; if (strlen(num) != 9) { return ""; } for (i = 0; i < 9; i++) { if (num[i]<’0’ && num[i]>’9’) { return ""; } } if (!strncmp(num, "601", 3) || !strncmp(num, "603", 3)) { return "M-ERA"; } if (!strncmp(num, "602", 3) || !strncmp(num, "604", 3)) { return "M-PLUS"; } if (!strncmp(num, "501", 3) || !strncmp(num, "502", 3)) { return "M-IDEA"; } area = 10*(num[0]-’0’) + num[1]-’0’; return areas[area]; } Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 5 / 23

slide-10
SLIDE 10

const char areas[][16] = {"","","","","","","","","","","","","Krakow",""....}; const char* phonearea(const char* num) { int i, area; if (strlen(num) != 9) { return ""; } for (i = 0; i < 9; i++) { if (num[i]<’0’ && num[i]>’9’) { return ""; } } if (!strncmp(num, "50", 2) || !strncmp(num, "60", 2) || !strncmp(num, "69", 2) || !strncmp(num, "51", 2)) { int res, op; res = mobile_db_lookup(num, &op); if (res == 0) { switch (op) { case 26001: return "T-ERA"; break; case 26002: return "T-PLUS"; break case 26003: return "T-ORANGE"; break; } } } if (!strncmp(num, "601") || !strncmp(num, "603")) { return "M-ERA"; } if (!strncmp(num, "602") || !strncmp(num, "604")) { return "M-PLUS"; } if (!strncmp(num, "501") || !strncmp(num, "502")) { return "M-IDEA"; } area = 10*(num[0]-’0’) + num[1]-’0’; return areas[area]; }

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 6 / 23

slide-11
SLIDE 11

What’s wrong with old code?

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 7 / 23

slide-12
SLIDE 12

What’s wrong with old code?

Not badly written - just filled with technical debt

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 7 / 23

slide-13
SLIDE 13

What’s wrong with old code?

Not badly written - just filled with technical debt No testability

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 7 / 23

slide-14
SLIDE 14

What’s wrong with old code?

Not badly written - just filled with technical debt No testability It works, it performs well - ”if it ain’t broken don’t fix it”

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 7 / 23

slide-15
SLIDE 15

What’s wrong with old code?

Not badly written - just filled with technical debt No testability It works, it performs well - ”if it ain’t broken don’t fix it” It won’t fix itself

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 7 / 23

slide-16
SLIDE 16

What’s wrong with old code?

Not badly written - just filled with technical debt No testability It works, it performs well - ”if it ain’t broken don’t fix it” It won’t fix itself Hard to understand

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 7 / 23

slide-17
SLIDE 17

What’s wrong with old code?

Not badly written - just filled with technical debt No testability It works, it performs well - ”if it ain’t broken don’t fix it” It won’t fix itself Hard to understand High barrier to entry

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 7 / 23

slide-18
SLIDE 18

BIND9

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 8 / 23

slide-19
SLIDE 19

BIND9

First commit (imported from CVS):

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 8 / 23

slide-20
SLIDE 20

BIND9

First commit (imported from CVS): commit 7ee52cc7d195433bb8f55972e2a8ab29668f7bce Author: Bob Halley <source@isc.org> Date: Mon Aug 17 22:05:58 1998 +0000

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 8 / 23

slide-21
SLIDE 21

BIND9

First commit (imported from CVS): commit 7ee52cc7d195433bb8f55972e2a8ab29668f7bce Author: Bob Halley <source@isc.org> Date: Mon Aug 17 22:05:58 1998 +0000 Replacement for BIND8

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 8 / 23

slide-22
SLIDE 22

BIND9

First commit (imported from CVS): commit 7ee52cc7d195433bb8f55972e2a8ab29668f7bce Author: Bob Halley <source@isc.org> Date: Mon Aug 17 22:05:58 1998 +0000 Replacement for BIND8 Buggy Internet Name Daemon

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 8 / 23

slide-23
SLIDE 23

BIND9

First commit (imported from CVS): commit 7ee52cc7d195433bb8f55972e2a8ab29668f7bce Author: Bob Halley <source@isc.org> Date: Mon Aug 17 22:05:58 1998 +0000 Replacement for BIND8 Buggy Internet Name Daemon Design by contract

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 8 / 23

slide-24
SLIDE 24

BIND9

First commit (imported from CVS): commit 7ee52cc7d195433bb8f55972e2a8ab29668f7bce Author: Bob Halley <source@isc.org> Date: Mon Aug 17 22:05:58 1998 +0000 Replacement for BIND8 Buggy Internet Name Daemon Design by contract No exploits

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 8 / 23

slide-25
SLIDE 25

BIND9

First commit (imported from CVS): commit 7ee52cc7d195433bb8f55972e2a8ab29668f7bce Author: Bob Halley <source@isc.org> Date: Mon Aug 17 22:05:58 1998 +0000 Replacement for BIND8 Buggy Internet Name Daemon Design by contract No exploits just possibility of DoS

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 8 / 23

slide-26
SLIDE 26

BIND9

First commit (imported from CVS): commit 7ee52cc7d195433bb8f55972e2a8ab29668f7bce Author: Bob Halley <source@isc.org> Date: Mon Aug 17 22:05:58 1998 +0000 Replacement for BIND8 Buggy Internet Name Daemon Design by contract No exploits just possibility of DoS BIND9 is older than:

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 8 / 23

slide-27
SLIDE 27

BIND9

First commit (imported from CVS): commit 7ee52cc7d195433bb8f55972e2a8ab29668f7bce Author: Bob Halley <source@isc.org> Date: Mon Aug 17 22:05:58 1998 +0000 Replacement for BIND8 Buggy Internet Name Daemon Design by contract No exploits just possibility of DoS BIND9 is older than:

◮ The Matrix (1999) Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 8 / 23

slide-28
SLIDE 28

BIND9

First commit (imported from CVS): commit 7ee52cc7d195433bb8f55972e2a8ab29668f7bce Author: Bob Halley <source@isc.org> Date: Mon Aug 17 22:05:58 1998 +0000 Replacement for BIND8 Buggy Internet Name Daemon Design by contract No exploits just possibility of DoS BIND9 is older than:

◮ The Matrix (1999) ◮ Agile Manifesto (2001) Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 8 / 23

slide-29
SLIDE 29

BIND9

First commit (imported from CVS): commit 7ee52cc7d195433bb8f55972e2a8ab29668f7bce Author: Bob Halley <source@isc.org> Date: Mon Aug 17 22:05:58 1998 +0000 Replacement for BIND8 Buggy Internet Name Daemon Design by contract No exploits just possibility of DoS BIND9 is older than:

◮ The Matrix (1999) ◮ Agile Manifesto (2001) ◮ Test Driven Development (2003) Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 8 / 23

slide-30
SLIDE 30

BIND9

First commit (imported from CVS): commit 7ee52cc7d195433bb8f55972e2a8ab29668f7bce Author: Bob Halley <source@isc.org> Date: Mon Aug 17 22:05:58 1998 +0000 Replacement for BIND8 Buggy Internet Name Daemon Design by contract No exploits just possibility of DoS BIND9 is older than:

◮ The Matrix (1999) ◮ Agile Manifesto (2001) ◮ Test Driven Development (2003) ◮ Linux 2.2 (1999) Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 8 / 23

slide-31
SLIDE 31

BIND9

First commit (imported from CVS): commit 7ee52cc7d195433bb8f55972e2a8ab29668f7bce Author: Bob Halley <source@isc.org> Date: Mon Aug 17 22:05:58 1998 +0000 Replacement for BIND8 Buggy Internet Name Daemon Design by contract No exploits just possibility of DoS BIND9 is older than:

◮ The Matrix (1999) ◮ Agile Manifesto (2001) ◮ Test Driven Development (2003) ◮ Linux 2.2 (1999) not to mention 2.6 with NPTL (2003) Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 8 / 23

slide-32
SLIDE 32

BIND is like an onion

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 9 / 23

slide-33
SLIDE 33

BIND is like an onion

Reference implementation

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 9 / 23

slide-34
SLIDE 34

BIND is like an onion

Reference implementation If there’s an RFC - BIND has it

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 9 / 23

slide-35
SLIDE 35

BIND is like an onion

Reference implementation If there’s an RFC - BIND has it Support for servers on dialup connections

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 9 / 23

slide-36
SLIDE 36

BIND is like an onion

Reference implementation If there’s an RFC - BIND has it Support for servers on dialup connections Small team and not many external contributors (barrier to entry)

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 9 / 23

slide-37
SLIDE 37

BIND is like an onion

Reference implementation If there’s an RFC - BIND has it Support for servers on dialup connections Small team and not many external contributors (barrier to entry) That’s how old and complex code is created

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 9 / 23

slide-38
SLIDE 38

LET’S DO SCIENCE!

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 10 / 23

slide-39
SLIDE 39

LET’S DO SCIENCE!

How to define complex code?

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 10 / 23

slide-40
SLIDE 40

LET’S DO SCIENCE!

How to define complex code? McCabe cyclomatic complexity

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 10 / 23

slide-41
SLIDE 41

LET’S DO SCIENCE!

How to define complex code? McCabe cyclomatic complexity Number of linearly independent paths through a program’s (function) source code

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 10 / 23

slide-42
SLIDE 42

LET’S DO SCIENCE!

How to define complex code? McCabe cyclomatic complexity Number of linearly independent paths through a program’s (function) source code The more complex code the harder it is to understand it, and the more impossible it becomes to test it

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 10 / 23

slide-43
SLIDE 43

LET’S DO SCIENCE!

How to define complex code? McCabe cyclomatic complexity Number of linearly independent paths through a program’s (function) source code The more complex code the harder it is to understand it, and the more impossible it becomes to test it Below 10 - OK

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 10 / 23

slide-44
SLIDE 44

LET’S DO SCIENCE!

How to define complex code? McCabe cyclomatic complexity Number of linearly independent paths through a program’s (function) source code The more complex code the harder it is to understand it, and the more impossible it becomes to test it Below 10 - OK Below 20 - Worrying

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 10 / 23

slide-45
SLIDE 45

LET’S DO SCIENCE!

How to define complex code? McCabe cyclomatic complexity Number of linearly independent paths through a program’s (function) source code The more complex code the harder it is to understand it, and the more impossible it becomes to test it Below 10 - OK Below 20 - Worrying Above 20 - Bad

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 10 / 23

slide-46
SLIDE 46

LET’S DO SCIENCE!

How to define complex code? McCabe cyclomatic complexity Number of linearly independent paths through a program’s (function) source code The more complex code the harder it is to understand it, and the more impossible it becomes to test it Below 10 - OK Below 20 - Worrying Above 20 - Bad Above 40 - Horrible

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 10 / 23

slide-47
SLIDE 47

LET’S DO APPLIED SCIENCE!

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 11 / 23

slide-48
SLIDE 48

LET’S DO APPLIED SCIENCE!

pmccabe - simple tool to calculate McCabe cyclomatic complexity of functions

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 11 / 23

slide-49
SLIDE 49

LET’S DO APPLIED SCIENCE!

pmccabe - simple tool to calculate McCabe cyclomatic complexity of functions BIND9 source code, bin/named/query.c

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 11 / 23

slide-50
SLIDE 50

LET’S DO APPLIED SCIENCE!

pmccabe - simple tool to calculate McCabe cyclomatic complexity of functions BIND9 source code, bin/named/query.c

static isc_result_t query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype);

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 11 / 23

slide-51
SLIDE 51

LET’S DO APPLIED SCIENCE!

pmccabe - simple tool to calculate McCabe cyclomatic complexity of functions BIND9 source code, bin/named/query.c

static isc_result_t query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype);

2500LOC

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 11 / 23

slide-52
SLIDE 52

LET’S DO APPLIED SCIENCE!

pmccabe - simple tool to calculate McCabe cyclomatic complexity of functions BIND9 source code, bin/named/query.c

static isc_result_t query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype);

2500LOC Complexity 474

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 11 / 23

slide-53
SLIDE 53

LET’S DO APPLIED SCIENCE!

pmccabe - simple tool to calculate McCabe cyclomatic complexity of functions BIND9 source code, bin/named/query.c

static isc_result_t query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype);

2500LOC Complexity 474 Tons of gotos, going backward, going into switch statements, etc.

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 11 / 23

slide-54
SLIDE 54

LET’S DO APPLIED SCIENCE!

pmccabe - simple tool to calculate McCabe cyclomatic complexity of functions BIND9 source code, bin/named/query.c

static isc_result_t query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype);

2500LOC Complexity 474 Tons of gotos, going backward, going into switch statements, etc. It wasn’t always that bad - started of at around 100

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 11 / 23

slide-55
SLIDE 55

LET’S DO APPLIED SCIENCE!

pmccabe - simple tool to calculate McCabe cyclomatic complexity of functions BIND9 source code, bin/named/query.c

static isc_result_t query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype);

2500LOC Complexity 474 Tons of gotos, going backward, going into switch statements, etc. It wasn’t always that bad - started of at around 100 Lots of mobile operators, lots of humps of the DNS camel...

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 11 / 23

slide-56
SLIDE 56

LET’S DO APPLIED SCIENCE!

pmccabe - simple tool to calculate McCabe cyclomatic complexity of functions BIND9 source code, bin/named/query.c

static isc_result_t query_find(ns_client_t *client, dns_fetchevent_t *event, dns_rdatatype_t qtype);

2500LOC Complexity 474 Tons of gotos, going backward, going into switch statements, etc. It wasn’t always that bad - started of at around 100 Lots of mobile operators, lots of humps of the DNS camel... Hold my beer!

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 11 / 23

slide-57
SLIDE 57

Disclaimer

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 12 / 23

slide-58
SLIDE 58

Disclaimer

I’m not saying that my approach is perfect

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 12 / 23

slide-59
SLIDE 59

Disclaimer

I’m not saying that my approach is perfect I’m not saying that my approach is even good

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 12 / 23

slide-60
SLIDE 60

Disclaimer

I’m not saying that my approach is perfect I’m not saying that my approach is even good Most of things I’ll state are obvious, but I really wish someone would tell me them before I started...

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 12 / 23

slide-61
SLIDE 61

Disclaimer

I’m not saying that my approach is perfect I’m not saying that my approach is even good Most of things I’ll state are obvious, but I really wish someone would tell me them before I started... Wise people learn from mistakes

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 12 / 23

slide-62
SLIDE 62

Disclaimer

I’m not saying that my approach is perfect I’m not saying that my approach is even good Most of things I’ll state are obvious, but I really wish someone would tell me them before I started... Wise people learn from mistakes, smart people learn from others mistakes

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 12 / 23

slide-63
SLIDE 63

Disclaimer

I’m not saying that my approach is perfect I’m not saying that my approach is even good Most of things I’ll state are obvious, but I really wish someone would tell me them before I started... Wise people learn from mistakes, smart people learn from others mistakes The following is a collection of my mistakes

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 12 / 23

slide-64
SLIDE 64

How to start?

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 13 / 23

slide-65
SLIDE 65

How to start?

Read the code

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 13 / 23

slide-66
SLIDE 66

How to start?

Read the code Read the code

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 13 / 23

slide-67
SLIDE 67

How to start?

Read the code Read the code Read the code once again

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 13 / 23

slide-68
SLIDE 68

How to start?

Read the code Read the code Read the code once again You probably won’t understand all the possible flows - Mr. McCabe predicted that - but you’ll see the ’outline’

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 13 / 23

slide-69
SLIDE 69

How to start?

Read the code Read the code Read the code once again You probably won’t understand all the possible flows - Mr. McCabe predicted that - but you’ll see the ’outline’ Don’t start unless for every piece of the code you can at least tell what it’s doing

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 13 / 23

slide-70
SLIDE 70

Crisis

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 14 / 23

slide-71
SLIDE 71

Crisis

It’s a lost cause, let’s rewrite it from scratch!

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 14 / 23

slide-72
SLIDE 72

Crisis

It’s a lost cause, let’s rewrite it from scratch! ... can you guarantee that what you’ll write will perform at least as well as what you have now?

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 14 / 23

slide-73
SLIDE 73

Crisis

It’s a lost cause, let’s rewrite it from scratch! ... can you guarantee that what you’ll write will perform at least as well as what you have now? ... can you guarantee that the behaviour won’t change?

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 14 / 23

slide-74
SLIDE 74

Crisis

It’s a lost cause, let’s rewrite it from scratch! ... can you guarantee that what you’ll write will perform at least as well as what you have now? ... can you guarantee that the behaviour won’t change? ... how many new bugs will you introduce?

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 14 / 23

slide-75
SLIDE 75

Crisis

It’s a lost cause, let’s rewrite it from scratch! ... can you guarantee that what you’ll write will perform at least as well as what you have now? ... can you guarantee that the behaviour won’t change? ... how many new bugs will you introduce? ... do you have enough tests to verify it?

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 14 / 23

slide-76
SLIDE 76

Crisis

It’s a lost cause, let’s rewrite it from scratch! ... can you guarantee that what you’ll write will perform at least as well as what you have now? ... can you guarantee that the behaviour won’t change? ... how many new bugs will you introduce? ... do you have enough tests to verify it? ... do you have the budget?

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 14 / 23

slide-77
SLIDE 77

Crisis

It’s a lost cause, let’s rewrite it from scratch! ... can you guarantee that what you’ll write will perform at least as well as what you have now? ... can you guarantee that the behaviour won’t change? ... how many new bugs will you introduce? ... do you have enough tests to verify it? ... do you have the budget?

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 14 / 23

slide-78
SLIDE 78

Making the progress

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 15 / 23

slide-79
SLIDE 79

Making the progress

Cut it into smaller pieces:

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 15 / 23

slide-80
SLIDE 80

Making the progress

Cut it into smaller pieces: take something that looks like a ’function’

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 15 / 23

slide-81
SLIDE 81

Making the progress

Cut it into smaller pieces: take something that looks like a ’function’ ... and move it to a separate function

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 15 / 23

slide-82
SLIDE 82

Making the progress

Cut it into smaller pieces: take something that looks like a ’function’ ... and move it to a separate function Optionally - create a state structure to pass between functions

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 15 / 23

slide-83
SLIDE 83

if (event != NULL) { /* * We’re returning from recursion. Restore the query context * and resume. */ want_restart = false; rpz_st = client->query.rpz_st; if (rpz_st != NULL && (rpz_st->state & DNS_RPZ_RECURSING) != 0) { CTRACE(ISC_LOG_DEBUG(3), "resume from RPZ recursion"); is_zone = rpz_st->q.is_zone; authoritative = rpz_st->q.authoritative; RESTORE(zone, rpz_st->q.zone); RESTORE(node, rpz_st->q.node); RESTORE(db, rpz_st->q.db); RESTORE(rdataset, rpz_st->q.rdataset); RESTORE(sigrdataset, rpz_st->q.sigrdataset); qtype = rpz_st->q.qtype; if (event->node != NULL) dns_db_detachnode(event->db, &event->node); SAVE(rpz_st->r.db, event->db); rpz_st->r.r_type = event->qtype; SAVE(rpz_st->r.r_rdataset, event->rdataset); query_putrdataset(client, &event->sigrdataset); (...) Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 16 / 23

slide-84
SLIDE 84

static isc_result_t query_resume(query_ctx_t *qctx) { isc_result_t result; dns_name_t *tname; isc_buffer_t b; qctx->want_restart = false; qctx->rpz_st = qctx->client->query.rpz_st; if (qctx->rpz_st != NULL && (qctx->rpz_st->state & DNS_RPZ_RECURSING) != 0) { CCTRACE(ISC_LOG_DEBUG(3), "resume from RPZ recursion"); qctx->is_zone = qctx->rpz_st->q.is_zone; qctx->authoritative = qctx->rpz_st->q.authoritative; RESTORE(qctx->zone, qctx->rpz_st->q.zone); RESTORE(qctx->node, qctx->rpz_st->q.node); RESTORE(qctx->db, qctx->rpz_st->q.db); RESTORE(qctx->rdataset, qctx->rpz_st->q.rdataset); RESTORE(qctx->sigrdataset, qctx->rpz_st->q.sigrdataset); qctx->qtype = qctx->rpz_st->q.qtype; if (qctx->event->node != NULL) dns_db_detachnode(qctx->event->db, &qctx->event->node); SAVE(qctx->rpz_st->r.db, qctx->event->db); qctx->rpz_st->r.r_type = qctx->event->qtype; SAVE(qctx->rpz_st->r.r_rdataset, qctx->event->rdataset); query_putrdataset(qctx->client, &qctx->event->sigrdataset); (...) Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 17 / 23

slide-85
SLIDE 85

Stick to your job

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 18 / 23

slide-86
SLIDE 86

Stick to your job

Remember that: Your job is not to optimize the code

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 18 / 23

slide-87
SLIDE 87

Stick to your job

Remember that: Your job is not to optimize the code Your job is not to fix horrible bugs

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 18 / 23

slide-88
SLIDE 88

Stick to your job

Remember that: Your job is not to optimize the code Your job is not to fix horrible bugs Your job is not to re-write pieces that look like they can be rewritten

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 18 / 23

slide-89
SLIDE 89

Stick to your job

Remember that: Your job is not to optimize the code Your job is not to fix horrible bugs Your job is not to re-write pieces that look like they can be rewritten Your job is to refactor the code by cutting the function into smaller pieces

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 18 / 23

slide-90
SLIDE 90

Stick to your job

Remember that: Your job is not to optimize the code Your job is not to fix horrible bugs Your job is not to re-write pieces that look like they can be rewritten Your job is to refactor the code by cutting the function into smaller pieces I know it’s tempting to do fix things, but it’s really not the time

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 18 / 23

slide-91
SLIDE 91

Stick to your job

Remember that: Your job is not to optimize the code Your job is not to fix horrible bugs Your job is not to re-write pieces that look like they can be rewritten Your job is to refactor the code by cutting the function into smaller pieces I know it’s tempting to do fix things, but it’s really not the time Make comments, write bug reports, put post-it notes on your monitor but do not try to fix anything now

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 18 / 23

slide-92
SLIDE 92

Don’t be smart

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 19 / 23

slide-93
SLIDE 93

Don’t be smart

If a code block looks vaguely similiar to another code block that you just cut out - make a second function that might have exactly the same content

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 19 / 23

slide-94
SLIDE 94

Don’t be smart

If a code block looks vaguely similiar to another code block that you just cut out - make a second function that might have exactly the same content If a function can be easily simplified - that’s fine, you’ll do it later

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 19 / 23

slide-95
SLIDE 95

Don’t be smart

If a code block looks vaguely similiar to another code block that you just cut out - make a second function that might have exactly the same content If a function can be easily simplified - that’s fine, you’ll do it later If some code is not reachable - you’ll cut it out later

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 19 / 23

slide-96
SLIDE 96

Don’t be smart

If a code block looks vaguely similiar to another code block that you just cut out - make a second function that might have exactly the same content If a function can be easily simplified - that’s fine, you’ll do it later If some code is not reachable - you’ll cut it out later tl;dr; be a dumb code-cutting monkey

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 19 / 23

slide-97
SLIDE 97

Work slowly

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 20 / 23

slide-98
SLIDE 98

Work slowly

You might miss something that’s important

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 20 / 23

slide-99
SLIDE 99

Work slowly

You might miss something that’s important You might not notice that the piece of code you just cut out has some side effects on the global function state

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 20 / 23

slide-100
SLIDE 100

Work slowly

You might miss something that’s important You might not notice that the piece of code you just cut out has some side effects on the global function state ...and only realize that something’s wrong 3 days later when DNS64 test fails for no apparent reason

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 20 / 23

slide-101
SLIDE 101

Work slowly

You might miss something that’s important You might not notice that the piece of code you just cut out has some side effects on the global function state ...and only realize that something’s wrong 3 days later when DNS64 test fails for no apparent reason ... and you have to start from the beginning because you can’t figure out what’s wrong

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 20 / 23

slide-102
SLIDE 102

Work slowly

You might miss something that’s important You might not notice that the piece of code you just cut out has some side effects on the global function state ...and only realize that something’s wrong 3 days later when DNS64 test fails for no apparent reason ... and you have to start from the beginning because you can’t figure out what’s wrong Cut one piece

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 20 / 23

slide-103
SLIDE 103

Work slowly

You might miss something that’s important You might not notice that the piece of code you just cut out has some side effects on the global function state ...and only realize that something’s wrong 3 days later when DNS64 test fails for no apparent reason ... and you have to start from the beginning because you can’t figure out what’s wrong Cut one piece - commit

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 20 / 23

slide-104
SLIDE 104

Work slowly

You might miss something that’s important You might not notice that the piece of code you just cut out has some side effects on the global function state ...and only realize that something’s wrong 3 days later when DNS64 test fails for no apparent reason ... and you have to start from the beginning because you can’t figure out what’s wrong Cut one piece - commit - compile

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 20 / 23

slide-105
SLIDE 105

Work slowly

You might miss something that’s important You might not notice that the piece of code you just cut out has some side effects on the global function state ...and only realize that something’s wrong 3 days later when DNS64 test fails for no apparent reason ... and you have to start from the beginning because you can’t figure out what’s wrong Cut one piece - commit - compile - run all possible tests

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 20 / 23

slide-106
SLIDE 106

Work slowly

You might miss something that’s important You might not notice that the piece of code you just cut out has some side effects on the global function state ...and only realize that something’s wrong 3 days later when DNS64 test fails for no apparent reason ... and you have to start from the beginning because you can’t figure out what’s wrong Cut one piece - commit - compile - run all possible tests Don’t take shortcuts or you’ll pay

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 20 / 23

slide-107
SLIDE 107

Write unit tests

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 21 / 23

slide-108
SLIDE 108

Write unit tests

You now have a bunch of small, simple, testable functions

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 21 / 23

slide-109
SLIDE 109

Write unit tests

You now have a bunch of small, simple, testable functions And you know how the code works and what to expect out of it

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 21 / 23

slide-110
SLIDE 110

Write unit tests

You now have a bunch of small, simple, testable functions And you know how the code works and what to expect out of it So drop everything and write unit tests for the functions you’ve just created

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 21 / 23

slide-111
SLIDE 111

Write unit tests

You now have a bunch of small, simple, testable functions And you know how the code works and what to expect out of it So drop everything and write unit tests for the functions you’ve just created seriously, there will never be the better time to do this

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 21 / 23

slide-112
SLIDE 112

Write unit tests

You now have a bunch of small, simple, testable functions And you know how the code works and what to expect out of it So drop everything and write unit tests for the functions you’ve just created seriously, there will never be the better time to do this Seriously, just do it - I didn’t and now I really regret it

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 21 / 23

slide-113
SLIDE 113

Taking care of the post-it notes

Now you can fix thing not worrying that you’ll break something

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 22 / 23

slide-114
SLIDE 114

Taking care of the post-it notes

Now you can fix thing not worrying that you’ll break something Now is the time to fix bugs you’ve found

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 22 / 23

slide-115
SLIDE 115

Taking care of the post-it notes

Now you can fix thing not worrying that you’ll break something Now is the time to fix bugs you’ve found Now is the time to optimize

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 22 / 23

slide-116
SLIDE 116

Taking care of the post-it notes

Now you can fix thing not worrying that you’ll break something Now is the time to fix bugs you’ve found Now is the time to optimize Now is the time to merge similiar functions

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 22 / 23

slide-117
SLIDE 117

Taking care of the post-it notes

Now you can fix thing not worrying that you’ll break something Now is the time to fix bugs you’ve found Now is the time to optimize Now is the time to merge similiar functions The result is probably not perfect, but it’s good enough to work on further

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 22 / 23

slide-118
SLIDE 118

Taking care of the post-it notes

Now you can fix thing not worrying that you’ll break something Now is the time to fix bugs you’ve found Now is the time to optimize Now is the time to merge similiar functions The result is probably not perfect, but it’s good enough to work on further Examples - qname minimization, modules

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 22 / 23

slide-119
SLIDE 119

Taking care of the post-it notes

Now you can fix thing not worrying that you’ll break something Now is the time to fix bugs you’ve found Now is the time to optimize Now is the time to merge similiar functions The result is probably not perfect, but it’s good enough to work on further Examples - qname minimization, modules Remember to measure your code regularly

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 22 / 23

slide-120
SLIDE 120

Questions?

Witold Kręcicki Stories from BIND9 refactoring February 3, 2019 23 / 23