Skip to content

Commit 1ff74ba

Browse files
yuvaltassacopybara-github
authored andcommitted
Change the type of state signature arguments from unsigned int to int
PiperOrigin-RevId: 843162492 Change-Id: I51d3324f088e8055898e115bdd32267c5950a412
1 parent fe0a11d commit 1ff74ba

File tree

9 files changed

+92
-46
lines changed

9 files changed

+92
-46
lines changed

doc/changelog.rst

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,15 @@ Upcoming version (not yet released)
77

88
General
99
^^^^^^^
10+
- Non-breaking ABI changes:
11+
12+
- The type of the ``sig`` (signature) argument of :ref:`mj_stateSize` and related functions has been changed from
13+
``unsigned int`` to ``int``. Before this change, invalid negative arguments passed to this function would result in
14+
a silent implicit cast, now negativity will trigger an error.
1015

1116
MJX
1217
^^^
13-
1. Added ``actuator_length``, ``cdof`` and ``cdof_dof`` fields to ``mjx.Data``.
18+
- Added ``actuator_length``, ``cdof`` and ``cdof_dof`` fields to ``mjx.Data``.
1419

1520
Bug fixes
1621
^^^^^^^^^

doc/includes/references.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3178,12 +3178,12 @@ void mj_projectConstraint(const mjModel* m, mjData* d);
31783178
void mj_referenceConstraint(const mjModel* m, mjData* d);
31793179
void mj_constraintUpdate(const mjModel* m, mjData* d, const mjtNum* jar,
31803180
mjtNum cost[1], int flg_coneHessian);
3181-
int mj_stateSize(const mjModel* m, unsigned int sig);
3182-
void mj_getState(const mjModel* m, const mjData* d, mjtNum* state, unsigned int sig);
3183-
void mj_extractState(const mjModel* m, const mjtNum* src, unsigned int srcsig,
3184-
mjtNum* dst, unsigned int dstsig);
3185-
void mj_setState(const mjModel* m, mjData* d, const mjtNum* state, unsigned int sig);
3186-
void mj_copyState(const mjModel* m, const mjData* src, mjData* dst, unsigned int sig);
3181+
int mj_stateSize(const mjModel* m, int sig);
3182+
void mj_getState(const mjModel* m, const mjData* d, mjtNum* state, int sig);
3183+
void mj_extractState(const mjModel* m, const mjtNum* src, int srcsig,
3184+
mjtNum* dst, int dstsig);
3185+
void mj_setState(const mjModel* m, mjData* d, const mjtNum* state, int sig);
3186+
void mj_copyState(const mjModel* m, const mjData* src, mjData* dst, int sig);
31873187
void mj_setKeyframe(mjModel* m, const mjData* d, int k);
31883188
int mj_addContact(const mjModel* m, mjData* d, const mjContact* con);
31893189
int mj_isPyramidal(const mjModel* m);

include/mujoco/mujoco.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -471,20 +471,20 @@ MJAPI void mj_constraintUpdate(const mjModel* m, mjData* d, const mjtNum* jar,
471471
//---------------------------------- Support -------------------------------------------------------
472472

473473
// Return size of state signature.
474-
MJAPI int mj_stateSize(const mjModel* m, unsigned int sig);
474+
MJAPI int mj_stateSize(const mjModel* m, int sig);
475475

476476
// Get state.
477-
MJAPI void mj_getState(const mjModel* m, const mjData* d, mjtNum* state, unsigned int sig);
477+
MJAPI void mj_getState(const mjModel* m, const mjData* d, mjtNum* state, int sig);
478478

479479
// Extract a subset of components from a state previously obtained via mj_getState.
480-
MJAPI void mj_extractState(const mjModel* m, const mjtNum* src, unsigned int srcsig,
481-
mjtNum* dst, unsigned int dstsig);
480+
MJAPI void mj_extractState(const mjModel* m, const mjtNum* src, int srcsig,
481+
mjtNum* dst, int dstsig);
482482

483483
// Set state.
484-
MJAPI void mj_setState(const mjModel* m, mjData* d, const mjtNum* state, unsigned int sig);
484+
MJAPI void mj_setState(const mjModel* m, mjData* d, const mjtNum* state, int sig);
485485

486486
// Copy state from src to dst.
487-
MJAPI void mj_copyState(const mjModel* m, const mjData* src, mjData* dst, unsigned int sig);
487+
MJAPI void mj_copyState(const mjModel* m, const mjData* src, mjData* dst, int sig);
488488

489489
// Copy current state to the k-th model keyframe.
490490
MJAPI void mj_setKeyframe(mjModel* m, const mjData* d, int k);

python/mujoco/introspect/functions.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2461,7 +2461,7 @@
24612461
),
24622462
FunctionParameterDecl(
24632463
name='sig',
2464-
type=ValueType(name='unsigned int'),
2464+
type=ValueType(name='int'),
24652465
),
24662466
),
24672467
doc='Return size of state signature.',
@@ -2491,7 +2491,7 @@
24912491
),
24922492
FunctionParameterDecl(
24932493
name='sig',
2494-
type=ValueType(name='unsigned int'),
2494+
type=ValueType(name='int'),
24952495
),
24962496
),
24972497
doc='Get state.',
@@ -2515,7 +2515,7 @@
25152515
),
25162516
FunctionParameterDecl(
25172517
name='srcsig',
2518-
type=ValueType(name='unsigned int'),
2518+
type=ValueType(name='int'),
25192519
),
25202520
FunctionParameterDecl(
25212521
name='dst',
@@ -2525,7 +2525,7 @@
25252525
),
25262526
FunctionParameterDecl(
25272527
name='dstsig',
2528-
type=ValueType(name='unsigned int'),
2528+
type=ValueType(name='int'),
25292529
),
25302530
),
25312531
doc='Extract a subset of components from a state previously obtained via mj_getState.', # pylint: disable=line-too-long
@@ -2555,7 +2555,7 @@
25552555
),
25562556
FunctionParameterDecl(
25572557
name='sig',
2558-
type=ValueType(name='unsigned int'),
2558+
type=ValueType(name='int'),
25592559
),
25602560
),
25612561
doc='Set state.',
@@ -2585,7 +2585,7 @@
25852585
),
25862586
FunctionParameterDecl(
25872587
name='sig',
2588-
type=ValueType(name='unsigned int'),
2588+
type=ValueType(name='int'),
25892589
),
25902590
),
25912591
doc='Copy state from src to dst.',

src/engine/engine_support.c

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,15 @@ static inline const mjtNum* mj_stateElemConstPtr(const mjModel* m, const mjData*
164164

165165

166166
// get size of state signature
167-
int mj_stateSize(const mjModel* m, unsigned int sig) {
167+
int mj_stateSize(const mjModel* m, int sig) {
168+
if (sig < 0) {
169+
mjERROR("invalid state signature %d < 0", sig);
170+
return 0;
171+
}
172+
168173
if (sig >= (1<<mjNSTATE)) {
169-
mjERROR("invalid state signature %u >= 2^mjNSTATE", sig);
174+
mjERROR("invalid state signature %d >= 2^mjNSTATE", sig);
175+
return 0;
170176
}
171177

172178
int size = 0;
@@ -182,9 +188,15 @@ int mj_stateSize(const mjModel* m, unsigned int sig) {
182188

183189

184190
// get state
185-
void mj_getState(const mjModel* m, const mjData* d, mjtNum* state, unsigned int sig) {
191+
void mj_getState(const mjModel* m, const mjData* d, mjtNum* state, int sig) {
192+
if (sig < 0) {
193+
mjERROR("invalid state signature %d < 0", sig);
194+
return;
195+
}
196+
186197
if (sig >= (1<<mjNSTATE)) {
187-
mjERROR("invalid state signature %u >= 2^mjNSTATE", sig);
198+
mjERROR("invalid state signature %d >= 2^mjNSTATE", sig);
199+
return;
188200
}
189201

190202
int adr = 0;
@@ -213,8 +225,17 @@ void mj_getState(const mjModel* m, const mjData* d, mjtNum* state, unsigned int
213225

214226

215227
// extract a sub-state from a state
216-
void mj_extractState(const mjModel* m, const mjtNum* src, unsigned int srcsig,
217-
mjtNum* dst, unsigned int dstsig) {
228+
void mj_extractState(const mjModel* m, const mjtNum* src, int srcsig, mjtNum* dst, int dstsig) {
229+
if (srcsig < 0) {
230+
mjERROR("invalid srcsig %d < 0", srcsig);
231+
return;
232+
}
233+
234+
if (srcsig >= (1<<mjNSTATE)) {
235+
mjERROR("invalid srcsig %d >= 2^mjNSTATE", srcsig);
236+
return;
237+
}
238+
218239
if ((srcsig & dstsig) != dstsig) {
219240
mjERROR("dstsig is not a subset of srcsig");
220241
return;
@@ -235,9 +256,15 @@ void mj_extractState(const mjModel* m, const mjtNum* src, unsigned int srcsig,
235256

236257

237258
// set state
238-
void mj_setState(const mjModel* m, mjData* d, const mjtNum* state, unsigned int sig) {
259+
void mj_setState(const mjModel* m, mjData* d, const mjtNum* state, int sig) {
260+
if (sig < 0) {
261+
mjERROR("invalid state signature %d < 0", sig);
262+
return;
263+
}
264+
239265
if (sig >= (1<<mjNSTATE)) {
240-
mjERROR("invalid state signature %u >= 2^mjNSTATE", sig);
266+
mjERROR("invalid state signature %d >= 2^mjNSTATE", sig);
267+
return;
241268
}
242269

243270
int adr = 0;
@@ -266,9 +293,15 @@ void mj_setState(const mjModel* m, mjData* d, const mjtNum* state, unsigned int
266293

267294

268295
// copy state from src to dst
269-
void mj_copyState(const mjModel* m, const mjData* src, mjData* dst, unsigned int sig) {
296+
void mj_copyState(const mjModel* m, const mjData* src, mjData* dst, int sig) {
297+
if (sig < 0) {
298+
mjERROR("invalid state signature %d < 0", sig);
299+
return;
300+
}
301+
270302
if (sig >= (1<<mjNSTATE)) {
271-
mjERROR("invalid state signature %u >= 2^mjNSTATE", sig);
303+
mjERROR("invalid state signature %d >= 2^mjNSTATE", sig);
304+
return;
272305
}
273306

274307
for (int i=0; i < mjNSTATE; i++) {

src/engine/engine_support.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,20 +36,20 @@ MJAPI extern const int mjCONDATA_SIZE[mjNCONDATA]; // TODO(tassa): expose in pu
3636
//-------------------------- get/set state ---------------------------------------------------------
3737

3838
// return size of state signature
39-
MJAPI int mj_stateSize(const mjModel* m, unsigned int sig);
39+
MJAPI int mj_stateSize(const mjModel* m, int sig);
4040

4141
// get state
42-
MJAPI void mj_getState(const mjModel* m, const mjData* d, mjtNum* state, unsigned int sig);
42+
MJAPI void mj_getState(const mjModel* m, const mjData* d, mjtNum* state, int sig);
4343

4444
// extract a sub-state from a state
45-
MJAPI void mj_extractState(const mjModel* m, const mjtNum* src, unsigned int srcsig,
46-
mjtNum* dst, unsigned int dstsig);
45+
MJAPI void mj_extractState(const mjModel* m, const mjtNum* src, int srcsig,
46+
mjtNum* dst, int dstsig);
4747

4848
// set state
49-
MJAPI void mj_setState(const mjModel* m, mjData* d, const mjtNum* state, unsigned int sig);
49+
MJAPI void mj_setState(const mjModel* m, mjData* d, const mjtNum* state, int sig);
5050

5151
// copy state from src to dst
52-
MJAPI void mj_copyState(const mjModel* m, const mjData* src, mjData* dst, unsigned int sig);
52+
MJAPI void mj_copyState(const mjModel* m, const mjData* src, mjData* dst, int sig);
5353

5454
// copy current state to the k-th model keyframe
5555
MJAPI void mj_setKeyframe(mjModel* m, const mjData* d, int k);

test/engine/engine_support_test.cc

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -844,14 +844,22 @@ TEST_F(SupportTest, ExtractState) {
844844
std::strncpy(last_error_msg, msg, sizeof(last_error_msg));
845845
++error_count;
846846
};
847+
847848
auto* old_mju_user_error = mju_user_error;
848849
mju_user_error = error_handler;
850+
849851
mj_extractState(model, nullptr, srcsig, nullptr, mjSTATE_QFRC_APPLIED);
850-
mju_user_error = old_mju_user_error;
851852
EXPECT_EQ(error_count, 1);
852853
EXPECT_EQ(std::string_view(last_error_msg),
853854
"mj_extractState: dstsig is not a subset of srcsig");
854855

856+
mj_extractState(model, nullptr, -1, nullptr, mjSTATE_QFRC_APPLIED);
857+
EXPECT_EQ(error_count, 2);
858+
EXPECT_EQ(std::string_view(last_error_msg),
859+
"mj_extractState: invalid srcsig -1 < 0");
860+
861+
mju_user_error = old_mju_user_error;
862+
855863
mj_deleteData(data);
856864
mj_deleteModel(model);
857865
}

unity/Runtime/Bindings/MjBindings.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6655,19 +6655,19 @@ public unsafe struct mjvFigure_ {
66556655
public static unsafe extern void mj_constraintUpdate(mjModel_* m, mjData_* d, double* jar, double* cost, int flg_coneHessian);
66566656

66576657
[DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)]
6658-
public static unsafe extern int mj_stateSize(mjModel_* m, uint sig);
6658+
public static unsafe extern int mj_stateSize(mjModel_* m, int sig);
66596659

66606660
[DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)]
6661-
public static unsafe extern void mj_getState(mjModel_* m, mjData_* d, double* state, uint sig);
6661+
public static unsafe extern void mj_getState(mjModel_* m, mjData_* d, double* state, int sig);
66626662

66636663
[DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)]
6664-
public static unsafe extern void mj_extractState(mjModel_* m, double* src, uint srcsig, double* dst, uint dstsig);
6664+
public static unsafe extern void mj_extractState(mjModel_* m, double* src, int srcsig, double* dst, int dstsig);
66656665

66666666
[DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)]
6667-
public static unsafe extern void mj_setState(mjModel_* m, mjData_* d, double* state, uint sig);
6667+
public static unsafe extern void mj_setState(mjModel_* m, mjData_* d, double* state, int sig);
66686668

66696669
[DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)]
6670-
public static unsafe extern void mj_copyState(mjModel_* m, mjData_* src, mjData_* dst, uint sig);
6670+
public static unsafe extern void mj_copyState(mjModel_* m, mjData_* src, mjData_* dst, int sig);
66716671

66726672
[DllImport("mujoco", CallingConvention = CallingConvention.Cdecl)]
66736673
public static unsafe extern void mj_setKeyframe(mjModel_* m, mjData_* d, int k);

wasm/codegen/generated/bindings.cc

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7935,7 +7935,7 @@ int mj_copyBack_wrapper(MjSpec& s, const MjModel& m) {
79357935
return mj_copyBack(s.get(), m.get());
79367936
}
79377937

7938-
void mj_copyState_wrapper(const MjModel& m, const MjData& src, MjData& dst, unsigned int sig) {
7938+
void mj_copyState_wrapper(const MjModel& m, const MjData& src, MjData& dst, int sig) {
79397939
mj_copyState(m.get(), src.get(), dst.get(), sig);
79407940
}
79417941

@@ -7979,7 +7979,7 @@ void mj_energyVel_wrapper(const MjModel& m, MjData& d) {
79797979
mj_energyVel(m.get(), d.get());
79807980
}
79817981

7982-
void mj_extractState_wrapper(const MjModel& m, const NumberArray& src, unsigned int srcsig, const val& dst, unsigned int dstsig) {
7982+
void mj_extractState_wrapper(const MjModel& m, const NumberArray& src, int srcsig, const val& dst, int dstsig) {
79837983
UNPACK_ARRAY(mjtNum, src);
79847984
UNPACK_VALUE(mjtNum, dst);
79857985
mj_extractState(m.get(), src_.data(), srcsig, dst_.data(), dstsig);
@@ -8039,7 +8039,7 @@ mjtNum mj_geomDistance_wrapper(const MjModel& m, const MjData& d, int geom1, int
80398039
return mj_geomDistance(m.get(), d.get(), geom1, geom2, distmax, fromto_.data());
80408040
}
80418041

8042-
void mj_getState_wrapper(const MjModel& m, const MjData& d, const val& state, unsigned int sig) {
8042+
void mj_getState_wrapper(const MjModel& m, const MjData& d, const val& state, int sig) {
80438043
UNPACK_VALUE(mjtNum, state);
80448044
CHECK_SIZE(state, mj_stateSize(m.get(), sig));
80458045
mj_getState(m.get(), d.get(), state_.data(), sig);
@@ -8363,7 +8363,7 @@ void mj_setKeyframe_wrapper(MjModel& m, const MjData& d, int k) {
83638363
mj_setKeyframe(m.get(), d.get(), k);
83648364
}
83658365

8366-
void mj_setState_wrapper(const MjModel& m, MjData& d, const NumberArray& state, unsigned int sig) {
8366+
void mj_setState_wrapper(const MjModel& m, MjData& d, const NumberArray& state, int sig) {
83678367
UNPACK_ARRAY(mjtNum, state);
83688368
CHECK_SIZE(state, mj_stateSize(m.get(), sig));
83698369
mj_setState(m.get(), d.get(), state_.data(), sig);
@@ -8397,7 +8397,7 @@ void mj_solveM2_wrapper(const MjModel& m, MjData& d, const val& x, const NumberA
83978397
mj_solveM2(m.get(), d.get(), x_.data(), y_.data(), sqrtInvD_.data(), n);
83988398
}
83998399

8400-
int mj_stateSize_wrapper(const MjModel& m, unsigned int sig) {
8400+
int mj_stateSize_wrapper(const MjModel& m, int sig) {
84018401
return mj_stateSize(m.get(), sig);
84028402
}
84038403

0 commit comments

Comments
 (0)