Change in libosmocore[master]: Use msgb helper instead of local #define for debug print
Max has posted comments on this change. ( https://gerrit.osmocom.org/12271 ) Change subject: Use msgb helper instead of local #define for debug print .. Patch Set 2: > Patch Set 2: Code-Review-1 > I can update this change, if you don't mind. Sure, go ahead. -- To view, visit https://gerrit.osmocom.org/12271 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6be778236eff8f2153f3113f9379ecfbec9052b Gerrit-Change-Number: 12271 Gerrit-PatchSet: 2 Gerrit-Owner: Max Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Pau Espin Pedrol Gerrit-Reviewer: Stefan Sperling Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Comment-Date: Wed, 19 Dec 2018 14:49:04 + Gerrit-HasComments: No Gerrit-HasLabels: No
Change in libosmocore[master]: Use msgb helper instead of local #define for debug print
Stefan Sperling has posted comments on this change. ( https://gerrit.osmocom.org/12271 ) Change subject: Use msgb helper instead of local #define for debug print .. Patch Set 2: > Patch Set 2: > > > Ok, if there is no known way to do it with spatch, why not to do it > > manually, by hands? What's the point of using spatch? Speed vs > > quality? > > > > I can update this change, if you don't mind. > > I agree with Vadim here I'm confused why VERIFY() is even a macro instead of a function. Anyway, I agree that just changing the macro body to use the function would be sufficient. Would it not? -- To view, visit https://gerrit.osmocom.org/12271 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6be778236eff8f2153f3113f9379ecfbec9052b Gerrit-Change-Number: 12271 Gerrit-PatchSet: 2 Gerrit-Owner: Max Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Pau Espin Pedrol Gerrit-Reviewer: Stefan Sperling Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Comment-Date: Wed, 19 Dec 2018 14:26:50 + Gerrit-HasComments: No Gerrit-HasLabels: No
Change in libosmocore[master]: Use msgb helper instead of local #define for debug print
Pau Espin Pedrol has posted comments on this change. ( https://gerrit.osmocom.org/12271 ) Change subject: Use msgb helper instead of local #define for debug print .. Patch Set 2: > Ok, if there is no known way to do it with spatch, why not to do it > manually, by hands? What's the point of using spatch? Speed vs > quality? > > I can update this change, if you don't mind. I agree with Vadim here -- To view, visit https://gerrit.osmocom.org/12271 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6be778236eff8f2153f3113f9379ecfbec9052b Gerrit-Change-Number: 12271 Gerrit-PatchSet: 2 Gerrit-Owner: Max Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Pau Espin Pedrol Gerrit-Reviewer: Stefan Sperling Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Comment-Date: Wed, 19 Dec 2018 13:59:20 + Gerrit-HasComments: No Gerrit-HasLabels: No
Change in libosmocore[master]: Use msgb helper instead of local #define for debug print
Vadim Yanitskiy has posted comments on this change. ( https://gerrit.osmocom.org/12271 ) Change subject: Use msgb helper instead of local #define for debug print .. Patch Set 2: Code-Review-1 > Unfortunately I haven't found a way to do this with spatch. > Feel free to add corrected variant of .spatch if you know how. Ok, if there is no known way to do it with spatch, why not to do it manually, by hands? What's the point of using spatch? Speed vs quality? I can update this change, if you don't mind. -- To view, visit https://gerrit.osmocom.org/12271 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6be778236eff8f2153f3113f9379ecfbec9052b Gerrit-Change-Number: 12271 Gerrit-PatchSet: 2 Gerrit-Owner: Max Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Pau Espin Pedrol Gerrit-Reviewer: Stefan Sperling Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Comment-Date: Wed, 19 Dec 2018 13:33:33 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in libosmocore[master]: Use msgb helper instead of local #define for debug print
Max has posted comments on this change. ( https://gerrit.osmocom.org/12271 ) Change subject: Use msgb helper instead of local #define for debug print .. Patch Set 2: This change is ready for review. -- To view, visit https://gerrit.osmocom.org/12271 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6be778236eff8f2153f3113f9379ecfbec9052b Gerrit-Change-Number: 12271 Gerrit-PatchSet: 2 Gerrit-Owner: Max Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Pau Espin Pedrol Gerrit-Reviewer: Stefan Sperling Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Comment-Date: Tue, 18 Dec 2018 16:27:43 + Gerrit-HasComments: No Gerrit-HasLabels: No
Change in libosmocore[master]: Use msgb helper instead of local #define for debug print
Max has posted comments on this change. ( https://gerrit.osmocom.org/12271 ) Change subject: Use msgb helper instead of local #define for debug print .. Patch Set 1: (1 comment) https://gerrit.osmocom.org/#/c/12271/1/tests/gsm0808/gsm0808_test.c File tests/gsm0808/gsm0808_test.c: https://gerrit.osmocom.org/#/c/12271/1/tests/gsm0808/gsm0808_test.c@40 PS1, Line 40: #define VERIFY(msg, data, len) \ > I would rather change this to: […] Unfortunately I haven't found a way to do this with spatch. Feel free to add corrected variant of .spatch if you know how. -- To view, visit https://gerrit.osmocom.org/12271 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6be778236eff8f2153f3113f9379ecfbec9052b Gerrit-Change-Number: 12271 Gerrit-PatchSet: 1 Gerrit-Owner: Max Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Pau Espin Pedrol Gerrit-Reviewer: Stefan Sperling Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Comment-Date: Thu, 13 Dec 2018 15:05:46 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in libosmocore[master]: Use msgb helper instead of local #define for debug print
Vadim Yanitskiy has posted comments on this change. ( https://gerrit.osmocom.org/12271 ) Change subject: Use msgb helper instead of local #define for debug print .. Patch Set 1: Code-Review-1 (1 comment) https://gerrit.osmocom.org/#/c/12271/1/tests/gsm0808/gsm0808_test.c File tests/gsm0808/gsm0808_test.c: https://gerrit.osmocom.org/#/c/12271/1/tests/gsm0808/gsm0808_test.c@40 PS1, Line 40: #define VERIFY(msg, data, len) \ > This one can be dropped then right? I would rather change this to: #define VERIFY(msg, data) \ if (!msgb_eq_l3_data_print(msg, data, ARRAY_SIZE(data))) \ abort(); -- To view, visit https://gerrit.osmocom.org/12271 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6be778236eff8f2153f3113f9379ecfbec9052b Gerrit-Change-Number: 12271 Gerrit-PatchSet: 1 Gerrit-Owner: Max Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Pau Espin Pedrol Gerrit-Reviewer: Stefan Sperling Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Comment-Date: Wed, 12 Dec 2018 20:41:55 + Gerrit-HasComments: Yes Gerrit-HasLabels: Yes
Change in libosmocore[master]: Use msgb helper instead of local #define for debug print
Max has posted comments on this change. ( https://gerrit.osmocom.org/12271 ) Change subject: Use msgb helper instead of local #define for debug print .. Patch Set 1: > Patch Set 1: Code-Review+2 > > I don't agree, I don't like leaving a define dangling in a commit, but > anyway, not that important. So far I haven't found a way to make this cleanup with spatch as well unfortunately and I'd rather have 2 separate patches than mix up manual and autogenerated changes. -- To view, visit https://gerrit.osmocom.org/12271 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6be778236eff8f2153f3113f9379ecfbec9052b Gerrit-Change-Number: 12271 Gerrit-PatchSet: 1 Gerrit-Owner: Max Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Pau Espin Pedrol Gerrit-Comment-Date: Wed, 12 Dec 2018 18:10:54 + Gerrit-HasComments: No Gerrit-HasLabels: No
Change in libosmocore[master]: Use msgb helper instead of local #define for debug print
Pau Espin Pedrol has posted comments on this change. ( https://gerrit.osmocom.org/12271 ) Change subject: Use msgb helper instead of local #define for debug print .. Patch Set 1: Code-Review+2 I don't agree, I don't like leaving a define dangling in a commit, but anyway, not that important. -- To view, visit https://gerrit.osmocom.org/12271 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6be778236eff8f2153f3113f9379ecfbec9052b Gerrit-Change-Number: 12271 Gerrit-PatchSet: 1 Gerrit-Owner: Max Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-Reviewer: Pau Espin Pedrol Gerrit-Comment-Date: Wed, 12 Dec 2018 16:23:04 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in libosmocore[master]: Use msgb helper instead of local #define for debug print
Max has posted comments on this change. ( https://gerrit.osmocom.org/12271 ) Change subject: Use msgb helper instead of local #define for debug print .. Patch Set 1: > Patch Set 1: > I know you wanted to split them based on spatch Exactly. > but this way adds extra verbosity to history and makes it more difficult to > revert for no good reason. I don't think it's actually a problem: reverting 2 patches would be 2 clicks instead of 1 (and we rarely have to do this anyway). I think keeping manual changes separated from automated code editing is worth it. -- To view, visit https://gerrit.osmocom.org/12271 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6be778236eff8f2153f3113f9379ecfbec9052b Gerrit-Change-Number: 12271 Gerrit-PatchSet: 1 Gerrit-Owner: Max Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Max Gerrit-CC: Pau Espin Pedrol Gerrit-Comment-Date: Wed, 12 Dec 2018 16:20:09 + Gerrit-HasComments: No Gerrit-HasLabels: No
Change in libosmocore[master]: Use msgb helper instead of local #define for debug print
Pau Espin Pedrol has posted comments on this change. ( https://gerrit.osmocom.org/12271 ) Change subject: Use msgb helper instead of local #define for debug print .. Patch Set 1: Ah it's done in next patch. I know you wanted to split them based on spatch, but this way adds extra verbosity to history and makes it more difficult to revert for no good reason. -- To view, visit https://gerrit.osmocom.org/12271 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6be778236eff8f2153f3113f9379ecfbec9052b Gerrit-Change-Number: 12271 Gerrit-PatchSet: 1 Gerrit-Owner: Max Gerrit-Reviewer: Jenkins Builder (102) Gerrit-CC: Pau Espin Pedrol Gerrit-Comment-Date: Wed, 12 Dec 2018 13:07:22 + Gerrit-HasComments: No Gerrit-HasLabels: No
Change in libosmocore[master]: Use msgb helper instead of local #define for debug print
Pau Espin Pedrol has posted comments on this change. ( https://gerrit.osmocom.org/12271 ) Change subject: Use msgb helper instead of local #define for debug print .. Patch Set 1: (1 comment) https://gerrit.osmocom.org/#/c/12271/1/tests/gsm0808/gsm0808_test.c File tests/gsm0808/gsm0808_test.c: https://gerrit.osmocom.org/#/c/12271/1/tests/gsm0808/gsm0808_test.c@40 PS1, Line 40: #define VERIFY(msg, data, len) \ This one can be dropped then right? -- To view, visit https://gerrit.osmocom.org/12271 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6be778236eff8f2153f3113f9379ecfbec9052b Gerrit-Change-Number: 12271 Gerrit-PatchSet: 1 Gerrit-Owner: Max Gerrit-CC: Jenkins Builder (102) Gerrit-CC: Pau Espin Pedrol Gerrit-Comment-Date: Wed, 12 Dec 2018 13:05:53 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in libosmocore[master]: Use msgb helper instead of local #define for debug print
Max has uploaded this change for review. ( https://gerrit.osmocom.org/12271
Change subject: Use msgb helper instead of local #define for debug print
..
Use msgb helper instead of local #define for debug print
This change was made using following spatch program:
@@
expression a, b, c;
@@
- VERIFY(a, b, c);
+ if (!msgb_eq_l3_data_print(a, b, c))
+ abort();
Which was applied as follows:
spatch --in-place --sp-file verif.spatch tests/gsm0808/gsm0808_test.c
Change-Id: Ib6be778236eff8f2153f3113f9379ecfbec9052b
---
M tests/gsm0808/gsm0808_test.c
1 file changed, 62 insertions(+), 31 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/71/12271/1
diff --git a/tests/gsm0808/gsm0808_test.c b/tests/gsm0808/gsm0808_test.c
index 63b8720..a31e7d4 100644
--- a/tests/gsm0808/gsm0808_test.c
+++ b/tests/gsm0808/gsm0808_test.c
@@ -116,7 +116,8 @@
msgb_v_put(in_msg, 0x23);
msg = gsm0808_create_layer3_2(in_msg, , NULL);
- VERIFY(msg, res, ARRAY_SIZE(res));
+ if (!msgb_eq_l3_data_print(msg, res, ARRAY_SIZE(res)))
+ abort();
msgb_free(msg);
msgb_free(in_msg);
}
@@ -152,7 +153,8 @@
msg = gsm0808_create_layer3_2(in_msg, , _list);
- VERIFY(msg, res, ARRAY_SIZE(res));
+ if (!msgb_eq_l3_data_print(msg, res, ARRAY_SIZE(res)))
+ abort();
msgb_free(msg);
msgb_free(in_msg);
@@ -165,7 +167,8 @@
printf("Testing creating Reset\n");
msg = gsm0808_create_reset();
- VERIFY(msg, res, ARRAY_SIZE(res));
+ if (!msgb_eq_l3_data_print(msg, res, ARRAY_SIZE(res)))
+ abort();
msgb_free(msg);
}
@@ -176,7 +179,8 @@
printf("Testing creating Reset Ack\n");
msg = gsm0808_create_reset_ack();
- VERIFY(msg, res, ARRAY_SIZE(res));
+ if (!msgb_eq_l3_data_print(msg, res, ARRAY_SIZE(res)))
+ abort();
msgb_free(msg);
}
@@ -188,7 +192,8 @@
printf("Testing creating Clear Command\n");
msg = gsm0808_create_clear_command(0x23);
- VERIFY(msg, res, ARRAY_SIZE(res));
+ if (!msgb_eq_l3_data_print(msg, res, ARRAY_SIZE(res)))
+ abort();
msgb_free(msg);
}
@@ -199,7 +204,8 @@
printf("Testing creating Clear Complete\n");
msg = gsm0808_create_clear_complete();
- VERIFY(msg, res, ARRAY_SIZE(res));
+ if (!msgb_eq_l3_data_print(msg, res, ARRAY_SIZE(res)))
+ abort();
msgb_free(msg);
}
@@ -234,12 +240,14 @@
printf("Testing creating Chipher Mode Command\n");
msg = gsm0808_create_cipher(, NULL);
OSMO_ASSERT(msg);
- VERIFY(msg, res, ARRAY_SIZE(res));
+ if (!msgb_eq_l3_data_print(msg, res, ARRAY_SIZE(res)))
+ abort();
msgb_free(msg);
msg = gsm0808_create_cipher(, _imeisv);
OSMO_ASSERT(msg);
- VERIFY(msg, res2, ARRAY_SIZE(res2));
+ if (!msgb_eq_l3_data_print(msg, res2, ARRAY_SIZE(res2)))
+ abort();
msgb_free(msg);
}
@@ -259,19 +267,22 @@
/* with l3 data */
msg = gsm0808_create_cipher_complete(l3, 4);
- VERIFY(msg, res1, ARRAY_SIZE(res1));
+ if (!msgb_eq_l3_data_print(msg, res1, ARRAY_SIZE(res1)))
+ abort();
msgb_free(msg);
/* with l3 data but short */
l3->len -= 1;
l3->tail -= 1;
msg = gsm0808_create_cipher_complete(l3, 4);
- VERIFY(msg, res2, ARRAY_SIZE(res2));
+ if (!msgb_eq_l3_data_print(msg, res2, ARRAY_SIZE(res2)))
+ abort();
msgb_free(msg);
/* without l3 data */
msg = gsm0808_create_cipher_complete(NULL, 4);
- VERIFY(msg, res2, ARRAY_SIZE(res2));
+ if (!msgb_eq_l3_data_print(msg, res2, ARRAY_SIZE(res2)))
+ abort();
msgb_free(msg);
@@ -308,7 +319,8 @@
printf("Testing creating Cipher Reject\n");
msg = gsm0808_create_cipher_reject(cause);
- VERIFY(msg, res, ARRAY_SIZE(res));
+ if (!msgb_eq_l3_data_print(msg, res, ARRAY_SIZE(res)))
+ abort();
parse_cipher_reject(msg, cause);
@@ -323,7 +335,8 @@
printf("Testing creating Cipher Reject (extended)\n");
msg = gsm0808_create_cipher_reject_ext(GSM0808_CAUSE_CLASS_INVAL,
cause);
- VERIFY(msg, res, ARRAY_SIZE(res));
+ if (!msgb_eq_l3_data_print(msg, res, ARRAY_SIZE(res)))
+ abort();
parse_cipher_reject(msg, cause);
@@ -342,12 +355,14 @@
printf("Testing creating CM U\n");
msg = gsm0808_create_classmark_update(, 1, , 1);
- VERIFY(msg, res, ARRAY_SIZE(res));
+ if (!msgb_eq_l3_data_print(msg, res, ARRAY_SIZE(res)))
+ abort();
msgb_free(msg);
msg = gsm0808_create_classmark_update(, 1, NULL, 0);
- VERIFY(msg, res2o, ARRAY_SIZE(res2o));
+ if (!msgb_eq_l3_data_print(msg, res2o,
