{"action":"created","issue":{"url":"https://api.github.com/repos/martylamb/nailgun/issues/72","labels_url":"https://api.github.com/repos/martylamb/nailgun/issues/72/labels{/name}","comments_url":"https://api.github.com/repos/martylamb/nailgun/issues/72/comments","events_url":"https://api.github.com/repos/martylamb/nailgun/issues/72/events","html_url":"https://github.com/martylamb/nailgun/pull/72","id":104309831,"number":72,"title":"Don't overwrite NAILGUN_TTY_FORMAT with isatty checks","user":{"login":"bhamiltoncx","id":671827,"avatar_url":"https://avatars.githubusercontent.com/u/671827?v=3","gravatar_id":"","url":"https://api.github.com/users/bhamiltoncx","html_url":"https://github.com/bhamiltoncx","followers_url":"https://api.github.com/users/bhamiltoncx/followers","following_url":"https://api.github.com/users/bhamiltoncx/following{/other_user}","gists_url":"https://api.github.com/users/bhamiltoncx/gists{/gist_id}","starred_url":"https://api.github.com/users/bhamiltoncx/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bhamiltoncx/subscriptions","organizations_url":"https://api.github.com/users/bhamiltoncx/orgs","repos_url":"https://api.github.com/users/bhamiltoncx/repos","events_url":"https://api.github.com/users/bhamiltoncx/events{/privacy}","received_events_url":"https://api.github.com/users/bhamiltoncx/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2015-09-01T16:52:14Z","updated_at":"2015-09-21T21:28:36Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/martylamb/nailgun/pulls/72","html_url":"https://github.com/martylamb/nailgun/pull/72","diff_url":"https://github.com/martylamb/nailgun/pull/72.diff","patch_url":"https://github.com/martylamb/nailgun/pull/72.patch"},"body":"I noticed this lovely (ab)use of C in `ng.c`:\r\n\r\n  #define NAILGUN_TTY_FORMAT \"NAILGUN_TTY_%d=%d\"\r\n  char isattybuf[] = NAILGUN_TTY_FORMAT;\r\n  sprintf(isattybuf, NAILGUN_TTY_FORMAT, i, isatty(i));\r\n\r\nThis actually overwrites the temporary C string literal created by the preprocessor (of which there may be one or many).\r\n\r\nIt's never a good idea to rely on overwriting C string literals, so this pull request makes it so `isattybuf` is an actual writable byte buffer with a known size."},"comment":{"url":"https://api.github.com/repos/martylamb/nailgun/issues/comments/142113605","html_url":"https://github.com/martylamb/nailgun/pull/72#issuecomment-142113605","issue_url":"https://api.github.com/repos/martylamb/nailgun/issues/72","id":142113605,"user":{"login":"dwightguth","id":5104541,"avatar_url":"https://avatars.githubusercontent.com/u/5104541?v=3","gravatar_id":"","url":"https://api.github.com/users/dwightguth","html_url":"https://github.com/dwightguth","followers_url":"https://api.github.com/users/dwightguth/followers","following_url":"https://api.github.com/users/dwightguth/following{/other_user}","gists_url":"https://api.github.com/users/dwightguth/gists{/gist_id}","starred_url":"https://api.github.com/users/dwightguth/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dwightguth/subscriptions","organizations_url":"https://api.github.com/users/dwightguth/orgs","repos_url":"https://api.github.com/users/dwightguth/repos","events_url":"https://api.github.com/users/dwightguth/events{/privacy}","received_events_url":"https://api.github.com/users/dwightguth/received_events","type":"User","site_admin":false},"created_at":"2015-09-21T21:28:36Z","updated_at":"2015-09-21T21:28:36Z","body":"[RV-Match](http://runtimeverification.com/match/) says that this is actually well defined. According to C11 section 6.7.9 paragraph 14, I quote:\r\n```\r\nAn array of character type may be initialized by a character string literal or UTF−8 string literal, optionally enclosed in braces.  Successive bytes of the string literal (including the terminating null character if there is room or if the array is of unknown size) initialize the elements of the array.\r\n```\r\n\r\nCombined with the section on array declarators (6.7.6.2), this would seem to imply that the string literal is used to initialize the array declared, but that the array declares a separate object with length equal to the length of the string literal. This is also consistent with the fact that isattybuf has no linkage, and therefore, when declared, allocates a unique object in the heap for itself, distinct from the object allocated by a string literal.\r\n\r\nIt is rather subtle, I will admit, however. For example, if we were to write the seemingly equivalent program:\r\n```\r\n#define NAILGUN_TTY_FORMAT \"NAILGUN_TTY_%d=%d\"\r\nchar *isattybuf = NAILGUN_TTY_FORMAT;\r\nsprintf(isattybuf, NAILGUN_TTY_FORMAT, i, isatty(i));\r\n```\r\nwhere isattybuf is declared as a pointer, this actually would in fact be undefined.\r\n\r\nThis is also consistent with the behavior of gcc, in which the former program succeeds but the latter program triggers a segfault because the string literal is allocated in read-only memory.\r\n\r\nAh, C... It can be quite difficult to write correct code in it, even when you know what you're doing, can't it?"}}