{"action":"created","issue":{"url":"https://api.github.com/repos/ocaml/ocaml/issues/1274","repository_url":"https://api.github.com/repos/ocaml/ocaml","labels_url":"https://api.github.com/repos/ocaml/ocaml/issues/1274/labels{/name}","comments_url":"https://api.github.com/repos/ocaml/ocaml/issues/1274/comments","events_url":"https://api.github.com/repos/ocaml/ocaml/issues/1274/events","html_url":"https://github.com/ocaml/ocaml/pull/1274","id":247341194,"number":1274,"title":"String_val: return 'const char *' when -safe-string is globally enabled","user":{"login":"yallop","id":1736496,"avatar_url":"https://avatars3.githubusercontent.com/u/1736496?v=4","gravatar_id":"","url":"https://api.github.com/users/yallop","html_url":"https://github.com/yallop","followers_url":"https://api.github.com/users/yallop/followers","following_url":"https://api.github.com/users/yallop/following{/other_user}","gists_url":"https://api.github.com/users/yallop/gists{/gist_id}","starred_url":"https://api.github.com/users/yallop/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/yallop/subscriptions","organizations_url":"https://api.github.com/users/yallop/orgs","repos_url":"https://api.github.com/users/yallop/repos","events_url":"https://api.github.com/users/yallop/events{/privacy}","received_events_url":"https://api.github.com/users/yallop/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2017-08-02T10:21:16Z","updated_at":"2017-08-03T06:45:18Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/ocaml/ocaml/pulls/1274","html_url":"https://github.com/ocaml/ocaml/pull/1274","diff_url":"https://github.com/ocaml/ocaml/pull/1274.diff","patch_url":"https://github.com/ocaml/ocaml/pull/1274.patch"},"body":"(There is some discussion about this change in [Mantis PR 7594](https://caml.inria.fr/mantis/view.php?id=7594).)\r\n\r\n#### Background & motivation\r\n\r\nSince #687 it's possible to enable `-safe-string` globally at configure time.  #1252 makes configure-time `-safe-string` the default.\r\n\r\nAt present, `-safe-string` only affects OCaml code; C stubs that mutate OCaml strings will continue to compile without error. This may lead to subtle bugs, not least since the compiler now takes string immutability into account during optimizations (see, e.g., #703).\r\n\r\n#### This change\r\n\r\nThis PR updates `String_val` to return `const char *` when `-safe-string` is enabled.  (When `-safe-string` is not enabled the type of `String_val` is unchanged.)  With typical settings, attempts to modify the result of `String_val` will result in a compiler warning, like this:\r\n\r\n```\r\nbar.c: In function ‘foo’:\r\n/usr/lib/ocaml/caml/mlvalues.h:256:23: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]\r\n #define String_val(x) ((const char *) Bp_val(x))\r\n                       ^\r\nbar.c:6:13: note: in expansion of macro ‘String_val’\r\n   char *p = String_val(s);\r\n             ^~~~~~~~~~\r\n```\r\n\r\nThis PR also adds a new macro `Bytes_val`, for accessing `bytes` values via `unsigned char *`.  Additionally, the types of several public functions have additional `const` qualifiers.\r\n\r\n#### Compatibility\r\nThe change to `String_val` is slightly backwards-incompatible (but probably less so than defaulting to `-safe-string` (#1252)).  I ran some tests on `opam-repository`: of the 134 packages that use `Safe_string`, 4 will not compile with this patch if the compiler is configured with `-safe-string`.  ([More details in Mantis PR 7594](https://caml.inria.fr/mantis/view.php?id=7594#c18135).)\r\n"},"comment":{"url":"https://api.github.com/repos/ocaml/ocaml/issues/comments/319883003","html_url":"https://github.com/ocaml/ocaml/pull/1274#issuecomment-319883003","issue_url":"https://api.github.com/repos/ocaml/ocaml/issues/1274","id":319883003,"user":{"login":"mshinwell","id":1315488,"avatar_url":"https://avatars0.githubusercontent.com/u/1315488?v=4","gravatar_id":"","url":"https://api.github.com/users/mshinwell","html_url":"https://github.com/mshinwell","followers_url":"https://api.github.com/users/mshinwell/followers","following_url":"https://api.github.com/users/mshinwell/following{/other_user}","gists_url":"https://api.github.com/users/mshinwell/gists{/gist_id}","starred_url":"https://api.github.com/users/mshinwell/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mshinwell/subscriptions","organizations_url":"https://api.github.com/users/mshinwell/orgs","repos_url":"https://api.github.com/users/mshinwell/repos","events_url":"https://api.github.com/users/mshinwell/events{/privacy}","received_events_url":"https://api.github.com/users/mshinwell/received_events","type":"User","site_admin":false},"created_at":"2017-08-03T06:45:18Z","updated_at":"2017-08-03T06:45:18Z","body":"Ah, I realised that we've forgotten one thing: updating the C interface section of the manual.  Could you do that?  OK with that change."}}
{"actor":{"display_login":"mshinwell"}}