Change in ...libosmocore[master]: tdef: fixup osmo_tdef_set()

2019-09-12 Thread pespin
pespin has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/15478 )

Change subject: tdef: fixup osmo_tdef_set()
..


Patch Set 1: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15478
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ia91c2f17e40fb9e79ffa5a7f28ce9c3605664402
Gerrit-Change-Number: 15478
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Thu, 12 Sep 2019 11:31:05 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment


Change in ...libosmocore[master]: tdef: fixup osmo_tdef_set()

2019-09-11 Thread laforge
laforge has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/c/libosmocore/+/15478 )

Change subject: tdef: fixup osmo_tdef_set()
..

tdef: fixup osmo_tdef_set()

I missed code review, so here are my comments in form of a follow-up patch
for Id56a1226d724a374f04231df85fe5b49ffd2c43c.

- Fix 'as_unit' arg name to 'val_unit' as in the C file and API doc.
- Explain rounding-up behavior of value conversion in API doc.
- Use osmo_tdef_get_entry() instead of a loop.

Related: OS#4190
Change-Id: Ia91c2f17e40fb9e79ffa5a7f28ce9c3605664402
---
M include/osmocom/core/tdef.h
M src/tdef.c
2 files changed, 7 insertions(+), 9 deletions(-)

Approvals:
  Jenkins Builder: Verified
  fixeria: Looks good to me, approved
  laforge: Looks good to me, approved



diff --git a/include/osmocom/core/tdef.h b/include/osmocom/core/tdef.h
index a1ad4cc..8155688 100644
--- a/include/osmocom/core/tdef.h
+++ b/include/osmocom/core/tdef.h
@@ -97,7 +97,7 @@
 unsigned long osmo_tdef_get(const struct osmo_tdef *tdefs, int T, enum 
osmo_tdef_unit as_unit,
long val_if_not_present);
 struct osmo_tdef *osmo_tdef_get_entry(struct osmo_tdef *tdefs, int T);
-int osmo_tdef_set(struct osmo_tdef *tdefs, int T, unsigned long val, enum 
osmo_tdef_unit as_unit);
+int osmo_tdef_set(struct osmo_tdef *tdefs, int T, unsigned long val, enum 
osmo_tdef_unit val_unit);

 /*! Using osmo_tdef for osmo_fsm_inst: array entry for a mapping of state 
numbers to timeout definitions.
  * For a usage example, see osmo_tdef_get_state_timeout() and 
test_tdef_state_timeout() in tdef_test.c. */
diff --git a/src/tdef.c b/src/tdef.c
index dfb47f6..ab6a51b 100644
--- a/src/tdef.c
+++ b/src/tdef.c
@@ -212,6 +212,7 @@
 }

 /*! Set value in entry matching T, converting val from val_unit to unit of T.
+ * The converted value is rounded up to the next integer value of T's unit and 
clamped to ULONG_MAX, or 0 if val == 0.
  * \param[in] tdefs  Array of timer definitions, last entry being fully zero.
  * \param[in] T  Timer number to set the value for.
  * \param[in] val  The new timer value to set.
@@ -220,14 +221,11 @@
  */
 int osmo_tdef_set(struct osmo_tdef *tdefs, int T, unsigned long val, enum 
osmo_tdef_unit val_unit)
 {
-   struct osmo_tdef *t;
-   osmo_tdef_for_each(t, tdefs) {
-   if (t->T == T) {
-   t->val = osmo_tdef_round(val, val_unit, t->unit);
-   return 0;
-   }
-   }
-   return -EEXIST;
+   struct osmo_tdef *t = osmo_tdef_get_entry(tdefs, T);
+   if (!t)
+   return -EEXIST;
+   t->val = osmo_tdef_round(val, val_unit, t->unit);
+   return 0;
 }

 /*! Using osmo_tdef for osmo_fsm_inst: find a given state's 
osmo_tdef_state_timeout entry.

--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15478
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ia91c2f17e40fb9e79ffa5a7f28ce9c3605664402
Gerrit-Change-Number: 15478
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: laforge 
Gerrit-MessageType: merged


Change in ...libosmocore[master]: tdef: fixup osmo_tdef_set()

2019-09-11 Thread laforge
laforge has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/15478 )

Change subject: tdef: fixup osmo_tdef_set()
..


Patch Set 1: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15478
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ia91c2f17e40fb9e79ffa5a7f28ce9c3605664402
Gerrit-Change-Number: 15478
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: laforge 
Gerrit-Comment-Date: Wed, 11 Sep 2019 06:26:38 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment


Change in ...libosmocore[master]: tdef: fixup osmo_tdef_set()

2019-09-10 Thread fixeria
fixeria has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/15478 )

Change subject: tdef: fixup osmo_tdef_set()
..


Patch Set 1: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15478
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ia91c2f17e40fb9e79ffa5a7f28ce9c3605664402
Gerrit-Change-Number: 15478
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Comment-Date: Wed, 11 Sep 2019 00:02:40 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment


Change in ...libosmocore[master]: tdef: fixup osmo_tdef_set()

2019-09-10 Thread neels
neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/libosmocore/+/15478


Change subject: tdef: fixup osmo_tdef_set()
..

tdef: fixup osmo_tdef_set()

I missed code review, so here are my comments in form of a follow-up patch
for Id56a1226d724a374f04231df85fe5b49ffd2c43c.

- Fix 'as_unit' arg name to 'val_unit' as in the C file and API doc.
- Explain rounding-up behavior of value conversion in API doc.
- Use osmo_tdef_get_entry() instead of a loop.

Related: OS#4190
Change-Id: Ia91c2f17e40fb9e79ffa5a7f28ce9c3605664402
---
M include/osmocom/core/tdef.h
M src/tdef.c
2 files changed, 7 insertions(+), 9 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/78/15478/1

diff --git a/include/osmocom/core/tdef.h b/include/osmocom/core/tdef.h
index a1ad4cc..8155688 100644
--- a/include/osmocom/core/tdef.h
+++ b/include/osmocom/core/tdef.h
@@ -97,7 +97,7 @@
 unsigned long osmo_tdef_get(const struct osmo_tdef *tdefs, int T, enum 
osmo_tdef_unit as_unit,
long val_if_not_present);
 struct osmo_tdef *osmo_tdef_get_entry(struct osmo_tdef *tdefs, int T);
-int osmo_tdef_set(struct osmo_tdef *tdefs, int T, unsigned long val, enum 
osmo_tdef_unit as_unit);
+int osmo_tdef_set(struct osmo_tdef *tdefs, int T, unsigned long val, enum 
osmo_tdef_unit val_unit);

 /*! Using osmo_tdef for osmo_fsm_inst: array entry for a mapping of state 
numbers to timeout definitions.
  * For a usage example, see osmo_tdef_get_state_timeout() and 
test_tdef_state_timeout() in tdef_test.c. */
diff --git a/src/tdef.c b/src/tdef.c
index dfb47f6..ab6a51b 100644
--- a/src/tdef.c
+++ b/src/tdef.c
@@ -212,6 +212,7 @@
 }

 /*! Set value in entry matching T, converting val from val_unit to unit of T.
+ * The converted value is rounded up to the next integer value of T's unit and 
clamped to ULONG_MAX, or 0 if val == 0.
  * \param[in] tdefs  Array of timer definitions, last entry being fully zero.
  * \param[in] T  Timer number to set the value for.
  * \param[in] val  The new timer value to set.
@@ -220,14 +221,11 @@
  */
 int osmo_tdef_set(struct osmo_tdef *tdefs, int T, unsigned long val, enum 
osmo_tdef_unit val_unit)
 {
-   struct osmo_tdef *t;
-   osmo_tdef_for_each(t, tdefs) {
-   if (t->T == T) {
-   t->val = osmo_tdef_round(val, val_unit, t->unit);
-   return 0;
-   }
-   }
-   return -EEXIST;
+   struct osmo_tdef *t = osmo_tdef_get_entry(tdefs, T);
+   if (!t)
+   return -EEXIST;
+   t->val = osmo_tdef_round(val, val_unit, t->unit);
+   return 0;
 }

 /*! Using osmo_tdef for osmo_fsm_inst: find a given state's 
osmo_tdef_state_timeout entry.

--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15478
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ia91c2f17e40fb9e79ffa5a7f28ce9c3605664402
Gerrit-Change-Number: 15478
Gerrit-PatchSet: 1
Gerrit-Owner: neels 
Gerrit-MessageType: newchange