[CRIU] [PATCH 2/2] c-lib: converted `char *`-args to `const char *`
Martin Wührer
martin.wuehrer at artech.at
Sat Jan 26 17:14:26 MSK 2019
On Thu, 2019-01-24 at 20:09 +0100, Adrian Reber wrote:
> On Thu, Jan 24, 2019 at 09:10:51AM -0800, Andrei Vagin wrote:
> > On Fri, Dec 21, 2018 at 08:00:21AM +0100, Martin Wührer wrote:
> > > As most of the `criu_(local_)*` functions already call `strdup()`,
> > > it is possible, to change the function signature to `const char *`.
> > >
> > > As the struct `criu_opts` already contains a `const char *
> > > service_binary`, also the member `service_address` is changed to
> > > `const char`.
> > >
> > > Additonally, also the function `criu_local_set_freeze_cgroup()` now
> > > calls `strdup()`.
> >
> > This patch changes the public API, will it break existing users?
>
> If it will break existing users we would need to increase the SO name of
> the library.
>
> Adrian
>
But the current implementation has some drawbacks. If it is used, I get the warning
warning: passing argument 1 of ‘criu_set_service_address’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
criu_set_service_address(bbb);
if the following c-program is compiled with `gcc-8 -Wall test.c -lcriu`:
#include <criu/criu.h>
const char * bbb = "/test/bbbb";
int main() {
criu_init_opts();
criu_set_service_address("/test/aaa");
criu_set_service_address(bbb);
criu_free_opts();
}
Furthermore, if I want to use the criu-rpc library in the following C++ program (compiled with `g++-8 -Wall test_criu.cpp -lcriu`):
#include <criu/criu.h>
#include <string>
const char* bbb="/test/bbb";
const std::string ccc="/test/ccc";
int main() {
criu_init_opts();
criu_set_service_address("/test/aaa");
criu_set_service_address(bbb);
criu_set_service_address(ccc.c_str());
criu_free_opts();
}
I get a warning even if I use a string literal and even errors if I use a const std::string and a const char * pointer for the service address:
test_criu.cpp:7:41: warning: ISO C++ forbids converting a string constant to ‘char*’ [-Wwrite-strings]
criu_set_service_address("/test/aaa");
^
test_criu.cpp:8:30: error: invalid conversion from ‘const char*’ to ‘char*’ [-fpermissive]
criu_set_service_address(bbb);
^~~
test_criu.cpp:9:39: error: invalid conversion from ‘const char*’ to ‘char*’ [-fpermissive]
criu_set_service_address(ccc.c_str());
~~~~~~~~~^~
In particular the service address, that is used in this example, would probably be constant and even known at compile time.
But it would be necessary to cast away the constness if the warning (or the errors if C++ is used) should be omitted and the -Wall option is used.
And the same applies for parent_images, the log_file, aso.
> > > Signed-off-by: Martin Wührer <martin.wuehrer at artech.at>
> > > ---
> > > lib/c/criu.c | 65 ++++++++++++++++++++++++++--------------------------
> > > lib/c/criu.h | 58 +++++++++++++++++++++++-----------------------
> > > 2 files changed, 62 insertions(+), 61 deletions(-)
> > >
> > > diff --git a/lib/c/criu.c b/lib/c/criu.c
> > > index 5318fc17..b11f3236 100644
> > > --- a/lib/c/criu.c
> > > +++ b/lib/c/criu.c
> > > @@ -25,7 +25,7 @@ struct criu_opts {
> > > int (*notify)(char *action, criu_notify_arg_t na);
> > > enum criu_service_comm service_comm;
> > > union {
> > > - char *service_address;
> > > + const char *service_address;
> > > int service_fd;
> > > const char *service_binary;
> > > };
> > > @@ -45,7 +45,7 @@ void criu_set_service_comm(enum criu_service_comm comm)
> > > criu_local_set_service_comm(global_opts, comm);
> > > }
> > >
> > > -void criu_local_set_service_address(criu_opts *opts, char *path)
> > > +void criu_local_set_service_address(criu_opts *opts, const char *path)
> > > {
> > > if (path)
> > > opts->service_address = path;
> > > @@ -53,7 +53,7 @@ void criu_local_set_service_address(criu_opts *opts, char *path)
> > > opts->service_address = CR_DEFAULT_SERVICE_ADDRESS;
> > > }
> > >
> > > -void criu_set_service_address(char *path)
> > > +void criu_set_service_address(const char *path)
> > > {
> > > criu_local_set_service_address(global_opts, path);
> > > }
> > > @@ -221,6 +221,7 @@ void criu_local_free_opts(criu_opts *opts)
> > > free(opts->rpc->cgroup_props);
> > > free(opts->rpc->parent_img);
> > > free(opts->rpc->root);
> > > + free(opts->rpc->freeze_cgroup);
> > > free(opts->rpc->log_file);
> > > free(opts->rpc);
> > > }
> > > @@ -312,12 +313,12 @@ void criu_set_images_dir_fd(int fd)
> > > criu_local_set_images_dir_fd(global_opts, fd);
> > > }
> > >
> > > -void criu_local_set_parent_images(criu_opts *opts, char *path)
> > > +void criu_local_set_parent_images(criu_opts *opts, const char *path)
> > > {
> > > opts->rpc->parent_img = strdup(path);
> > > }
> > >
> > > -void criu_set_parent_images(char *path)
> > > +void criu_set_parent_images(const char *path)
> > > {
> > > criu_local_set_parent_images(global_opts, path);
> > > }
> > > @@ -523,12 +524,12 @@ void criu_set_log_level(int log_level)
> > > criu_local_set_log_level(global_opts, log_level);
> > > }
> > >
> > > -void criu_local_set_root(criu_opts *opts, char *root)
> > > +void criu_local_set_root(criu_opts *opts, const char *root)
> > > {
> > > opts->rpc->root = strdup(root);
> > > }
> > >
> > > -void criu_set_root(char *root)
> > > +void criu_set_root(const char *root)
> > > {
> > > criu_local_set_root(global_opts, root);
> > > }
> > > @@ -555,12 +556,12 @@ void criu_set_manage_cgroups_mode(enum criu_cg_mode mode)
> > > criu_local_set_manage_cgroups_mode(global_opts, mode);
> > > }
> > >
> > > -void criu_local_set_freeze_cgroup(criu_opts *opts, char *name)
> > > +void criu_local_set_freeze_cgroup(criu_opts *opts, const char *name)
> > > {
> > > - opts->rpc->freeze_cgroup = name;
> > > + opts->rpc->freeze_cgroup = strdup(name);
> > > }
> > >
> > > -void criu_set_freeze_cgroup(char *name)
> > > +void criu_set_freeze_cgroup(const char *name)
> > > {
> > > criu_local_set_freeze_cgroup(global_opts, name);
> > > }
> > > @@ -608,12 +609,12 @@ void criu_set_ext_masters(bool val)
> > > criu_local_set_ext_masters(global_opts, val);
> > > }
> > >
> > > -void criu_local_set_log_file(criu_opts *opts, char *log_file)
> > > +void criu_local_set_log_file(criu_opts *opts, const char *log_file)
> > > {
> > > opts->rpc->log_file = strdup(log_file);
> > > }
> > >
> > > -void criu_set_log_file(char *log_file)
> > > +void criu_set_log_file(const char *log_file)
> > > {
> > > criu_local_set_log_file(global_opts, log_file);
> > > }
> > > @@ -660,7 +661,7 @@ int criu_set_exec_cmd(int argc, char *argv[])
> > > return criu_local_set_exec_cmd(global_opts, argc, argv);
> > > }
> > >
> > > -int criu_local_add_ext_mount(criu_opts *opts, char *key, char *val)
> > > +int criu_local_add_ext_mount(criu_opts *opts, const char *key, const char *val)
> > > {
> > > int nr;
> > > ExtMountMap **a, *m;
> > > @@ -697,12 +698,12 @@ er:
> > > return -ENOMEM;
> > > }
> > >
> > > -int criu_add_ext_mount(char *key, char *val)
> > > +int criu_add_ext_mount(const char *key, const char *val)
> > > {
> > > return criu_local_add_ext_mount(global_opts, key, val);
> > > }
> > >
> > > -int criu_local_add_cg_root(criu_opts *opts, char *ctrl, char *path)
> > > +int criu_local_add_cg_root(criu_opts *opts, const char *ctrl, const char *path)
> > > {
> > > int nr;
> > > CgroupRoot **a, *root;
> > > @@ -743,12 +744,12 @@ er:
> > > return -ENOMEM;
> > > }
> > >
> > > -int criu_add_cg_root(char *ctrl, char *path)
> > > +int criu_add_cg_root(const char *ctrl, const char *path)
> > > {
> > > return criu_local_add_cg_root(global_opts, ctrl, path);
> > > }
> > >
> > > -int criu_local_add_veth_pair(criu_opts *opts, char *in, char *out)
> > > +int criu_local_add_veth_pair(criu_opts *opts, const char *in, const char *out)
> > > {
> > > int nr;
> > > CriuVethPair **a, *p;
> > > @@ -785,12 +786,12 @@ er:
> > > return -ENOMEM;
> > > }
> > >
> > > -int criu_add_veth_pair(char *in, char *out)
> > > +int criu_add_veth_pair(const char *in, const char *out)
> > > {
> > > return criu_local_add_veth_pair(global_opts, in, out);
> > > }
> > >
> > > -int criu_local_add_enable_fs(criu_opts *opts, char *fs)
> > > +int criu_local_add_enable_fs(criu_opts *opts, const char *fs)
> > > {
> > > int nr;
> > > char *str = NULL;
> > > @@ -821,13 +822,13 @@ err:
> > > return -ENOMEM;
> > > }
> > >
> > > -int criu_add_enable_fs(char *fs)
> > > +int criu_add_enable_fs(const char *fs)
> > > {
> > > return criu_local_add_enable_fs(global_opts, fs);
> > > }
> > >
> > >
> > > -int criu_local_add_skip_mnt(criu_opts *opts, char *mnt)
> > > +int criu_local_add_skip_mnt(criu_opts *opts, const char *mnt)
> > > {
> > > int nr;
> > > char *str = NULL;
> > > @@ -858,7 +859,7 @@ err:
> > > return -ENOMEM;
> > > }
> > >
> > > -int criu_local_add_irmap_path(criu_opts *opts, char *path)
> > > +int criu_local_add_irmap_path(criu_opts *opts, const char *path)
> > > {
> > > int nr;
> > > char *my_path;
> > > @@ -890,7 +891,7 @@ err:
> > > return -ENOMEM;
> > > }
> > >
> > > -int criu_local_add_cg_props(criu_opts *opts, char *stream)
> > > +int criu_local_add_cg_props(criu_opts *opts, const char *stream)
> > > {
> > > char *new;
> > >
> > > @@ -903,7 +904,7 @@ int criu_local_add_cg_props(criu_opts *opts, char *stream)
> > > return 0;
> > > }
> > >
> > > -int criu_local_add_cg_props_file(criu_opts *opts, char *path)
> > > +int criu_local_add_cg_props_file(criu_opts *opts, const char *path)
> > > {
> > > char *new;
> > >
> > > @@ -916,7 +917,7 @@ int criu_local_add_cg_props_file(criu_opts *opts, char *path)
> > > return 0;
> > > }
> > >
> > > -int criu_local_add_cg_dump_controller(criu_opts *opts, char *name)
> > > +int criu_local_add_cg_dump_controller(criu_opts *opts, const char *name)
> > > {
> > > char **new, *ctrl_name;
> > > size_t nr;
> > > @@ -940,7 +941,7 @@ int criu_local_add_cg_dump_controller(criu_opts *opts, char *name)
> > > return 0;
> > > }
> > >
> > > -int criu_add_skip_mnt(char *mnt)
> > > +int criu_add_skip_mnt(const char *mnt)
> > > {
> > > return criu_local_add_skip_mnt(global_opts, mnt);
> > > }
> > > @@ -956,12 +957,12 @@ void criu_set_ghost_limit(unsigned int limit)
> > > criu_local_set_ghost_limit(global_opts, limit);
> > > }
> > >
> > > -int criu_add_irmap_path(char *path)
> > > +int criu_add_irmap_path(const char *path)
> > > {
> > > return criu_local_add_irmap_path(global_opts, path);
> > > }
> > >
> > > -int criu_local_add_inherit_fd(criu_opts *opts, int fd, char *key)
> > > +int criu_local_add_inherit_fd(criu_opts *opts, int fd, const char *key)
> > > {
> > > int nr;
> > > InheritFd **a, *f;
> > > @@ -997,12 +998,12 @@ er:
> > > return -ENOMEM;
> > > }
> > >
> > > -int criu_add_inherit_fd(int fd, char *key)
> > > +int criu_add_inherit_fd(int fd, const char *key)
> > > {
> > > return criu_local_add_inherit_fd(global_opts, fd, key);
> > > }
> > >
> > > -int criu_local_add_external(criu_opts *opts, char *key)
> > > +int criu_local_add_external(criu_opts *opts, const char *key)
> > > {
> > > int nr;
> > > char **a, *e = NULL;
> > > @@ -1026,7 +1027,7 @@ err:
> > > return -ENOMEM;
> > > }
> > >
> > > -int criu_add_external(char *key)
> > > +int criu_add_external(const char *key)
> > > {
> > > return criu_local_add_external(global_opts, key);
> > > }
> > > @@ -1498,7 +1499,7 @@ int criu_local_restore_child(criu_opts *opts)
> > > {
> > > int sk, ret = -1;
> > > enum criu_service_comm saved_comm;
> > > - char *saved_comm_data;
> > > + const char *saved_comm_data;
> > > bool save_comm;
> > > CriuReq req = CRIU_REQ__INIT;
> > > CriuResp *resp = NULL;
> > > diff --git a/lib/c/criu.h b/lib/c/criu.h
> > > index e244be85..12508c81 100644
> > > --- a/lib/c/criu.h
> > > +++ b/lib/c/criu.h
> > > @@ -41,7 +41,7 @@ enum criu_cg_mode {
> > > CRIU_CG_MODE_DEFAULT,
> > > };
> > >
> > > -void criu_set_service_address(char *path);
> > > +void criu_set_service_address(const char *path);
> > > void criu_set_service_fd(int fd);
> > > void criu_set_service_binary(const char *path);
> > >
> > > @@ -60,7 +60,7 @@ void criu_free_opts(void);
> > >
> > > void criu_set_pid(int pid);
> > > void criu_set_images_dir_fd(int fd); /* must be set for dump/restore */
> > > -void criu_set_parent_images(char *path);
> > > +void criu_set_parent_images(const char *path);
> > > void criu_set_work_dir_fd(int fd);
> > > void criu_set_leave_running(bool leave_running);
> > > void criu_set_ext_unix_sk(bool ext_unix_sk);
> > > @@ -76,26 +76,26 @@ void criu_set_auto_dedup(bool auto_dedup);
> > > void criu_set_force_irmap(bool force_irmap);
> > > void criu_set_link_remap(bool link_remap);
> > > void criu_set_log_level(int log_level);
> > > -void criu_set_log_file(char *log_file);
> > > +void criu_set_log_file(const char *log_file);
> > > void criu_set_cpu_cap(unsigned int cap);
> > > -void criu_set_root(char *root);
> > > +void criu_set_root(const char *root);
> > > void criu_set_manage_cgroups(bool manage);
> > > void criu_set_manage_cgroups_mode(enum criu_cg_mode mode);
> > > -void criu_set_freeze_cgroup(char *name);
> > > +void criu_set_freeze_cgroup(const char *name);
> > > void criu_set_timeout(unsigned int timeout);
> > > void criu_set_auto_ext_mnt(bool val);
> > > void criu_set_ext_sharing(bool val);
> > > void criu_set_ext_masters(bool val);
> > > int criu_set_exec_cmd(int argc, char *argv[]);
> > > -int criu_add_ext_mount(char *key, char *val);
> > > -int criu_add_veth_pair(char *in, char *out);
> > > -int criu_add_cg_root(char *ctrl, char *path);
> > > -int criu_add_enable_fs(char *fs);
> > > -int criu_add_skip_mnt(char *mnt);
> > > +int criu_add_ext_mount(const char *key, const char *val);
> > > +int criu_add_veth_pair(const char *in, const char *out);
> > > +int criu_add_cg_root(const char *ctrl, const char *path);
> > > +int criu_add_enable_fs(const char *fs);
> > > +int criu_add_skip_mnt(const char *mnt);
> > > void criu_set_ghost_limit(unsigned int limit);
> > > -int criu_add_irmap_path(char *path);
> > > -int criu_add_inherit_fd(int fd, char *key);
> > > -int criu_add_external(char *key);
> > > +int criu_add_irmap_path(const char *path);
> > > +int criu_add_inherit_fd(int fd, const char *key);
> > > +int criu_add_external(const char *key);
> > >
> > > /*
> > > * The criu_notify_arg_t na argument is an opaque
> > > @@ -161,7 +161,7 @@ typedef struct criu_opts criu_opts;
> > > int criu_local_init_opts(criu_opts **opts);
> > > void criu_local_free_opts(criu_opts *opts);
> > >
> > > -void criu_local_set_service_address(criu_opts *opts, char *path);
> > > +void criu_local_set_service_address(criu_opts *opts, const char *path);
> > > void criu_local_set_service_fd(criu_opts *opts, int fd);
> > > void criu_local_set_service_comm(criu_opts *opts, enum criu_service_comm);
> > >
> > > @@ -169,7 +169,7 @@ void criu_local_set_service_fd(criu_opts *opts, int fd);
> > >
> > > void criu_local_set_pid(criu_opts *opts, int pid);
> > > void criu_local_set_images_dir_fd(criu_opts *opts, int fd); /* must be set for dump/restore */
> > > -void criu_local_set_parent_images(criu_opts *opts, char *path);
> > > +void criu_local_set_parent_images(criu_opts *opts, const char *path);
> > > void criu_local_set_service_binary(criu_opts *opts, const char *path);
> > > void criu_local_set_work_dir_fd(criu_opts *opts, int fd);
> > > void criu_local_set_leave_running(criu_opts *opts, bool leave_running);
> > > @@ -186,29 +186,29 @@ void criu_local_set_auto_dedup(criu_opts *opts, bool auto_dedup);
> > > void criu_local_set_force_irmap(criu_opts *opts, bool force_irmap);
> > > void criu_local_set_link_remap(criu_opts *opts, bool link_remap);
> > > void criu_local_set_log_level(criu_opts *opts, int log_level);
> > > -void criu_local_set_log_file(criu_opts *opts, char *log_file);
> > > +void criu_local_set_log_file(criu_opts *opts, const char *log_file);
> > > void criu_local_set_cpu_cap(criu_opts *opts, unsigned int cap);
> > > -void criu_local_set_root(criu_opts *opts, char *root);
> > > +void criu_local_set_root(criu_opts *opts, const char *root);
> > > void criu_local_set_manage_cgroups(criu_opts *opts, bool manage);
> > > void criu_local_set_manage_cgroups_mode(criu_opts *opts, enum criu_cg_mode mode);
> > > -void criu_local_set_freeze_cgroup(criu_opts *opts, char *name);
> > > +void criu_local_set_freeze_cgroup(criu_opts *opts, const char *name);
> > > void criu_local_set_timeout(criu_opts *opts, unsigned int timeout);
> > > void criu_local_set_auto_ext_mnt(criu_opts *opts, bool val);
> > > void criu_local_set_ext_sharing(criu_opts *opts, bool val);
> > > void criu_local_set_ext_masters(criu_opts *opts, bool val);
> > > int criu_local_set_exec_cmd(criu_opts *opts, int argc, char *argv[]);
> > > -int criu_local_add_ext_mount(criu_opts *opts, char *key, char *val);
> > > -int criu_local_add_veth_pair(criu_opts *opts, char *in, char *out);
> > > -int criu_local_add_cg_root(criu_opts *opts, char *ctrl, char *path);
> > > -int criu_local_add_enable_fs(criu_opts *opts, char *fs);
> > > -int criu_local_add_skip_mnt(criu_opts *opts, char *mnt);
> > > +int criu_local_add_ext_mount(criu_opts *opts, const char *key, const char *val);
> > > +int criu_local_add_veth_pair(criu_opts *opts, const char *in, const char *out);
> > > +int criu_local_add_cg_root(criu_opts *opts, const char *ctrl, const char *path);
> > > +int criu_local_add_enable_fs(criu_opts *opts, const char *fs);
> > > +int criu_local_add_skip_mnt(criu_opts *opts, const char *mnt);
> > > void criu_local_set_ghost_limit(criu_opts *opts, unsigned int limit);
> > > -int criu_local_add_irmap_path(criu_opts *opts, char *path);
> > > -int criu_local_add_cg_props(criu_opts *opts, char *stream);
> > > -int criu_local_add_cg_props_file(criu_opts *opts, char *path);
> > > -int criu_local_add_cg_dump_controller(criu_opts *opts, char *name);
> > > -int criu_local_add_inherit_fd(criu_opts *opts, int fd, char *key);
> > > -int criu_local_add_external(criu_opts *opts, char *key);
> > > +int criu_local_add_irmap_path(criu_opts *opts, const char *path);
> > > +int criu_local_add_cg_props(criu_opts *opts, const char *stream);
> > > +int criu_local_add_cg_props_file(criu_opts *opts, const char *path);
> > > +int criu_local_add_cg_dump_controller(criu_opts *opts, const char *name);
> > > +int criu_local_add_inherit_fd(criu_opts *opts, int fd, const char *key);
> > > +int criu_local_add_external(criu_opts *opts, const char *key);
> > >
> > > void criu_local_set_notify_cb(criu_opts *opts, int (*cb)(char *action, criu_notify_arg_t na));
> > >
> > > --
> > > 2.19.2
> > >
> > > _______________________________________________
> > > CRIU mailing list
> > > CRIU at openvz.org
> > > https://lists.openvz.org/mailman/listinfo/criu
> > _______________________________________________
> > CRIU mailing list
> > CRIU at openvz.org
> > https://lists.openvz.org/mailman/listinfo/criu
More information about the CRIU
mailing list