{"thread":{"id":"7412","subject":"Type mismatches in safe_read and friends?","startedAt":"2007-03-26T14:13:10Z","lastAt":"2007-03-27T10:34:03Z","messageCount":3,"participants":["Rogan Dawes","Shawn O. Pearce"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"38033","messageId":"4607D4F6.80703@dawes.za.net","threadId":"7412","inReplyTo":null,"subject":"Type mismatches in safe_read and friends?","fromName":"Rogan Dawes","fromEmail":"lists@dawes.za.net","sentAt":"2007-03-26T14:13:10Z","receivedAt":"2007-03-26T14:13:10Z","isPatch":false,"sender":{"key":"lists@dawes.za.net","avatar":null},"body":"Hi folks,\n\nI'm starting to learn a little C, and I figured I'd learn from the \nmasters ;-) I needed to read in some data from the network, and I \nfigured the safe_* calls would be a good example of how to do it correctly.\n\nSo, I took a look, and found:\n\nstatic void safe_read(int fd, void *buffer, unsigned size)\n{\n         int n = 0;\n\n         while (n < size) {\n                 int ret = xread(fd, (char *) buffer + n, size - n);\n                 if (ret < 0)\n                         die(\"read error (%s)\", strerror(errno));\n                 if (!ret)\n                         die(\"The remote end hung up unexpectedly\");\n                 n += ret;\n         }\n}\n\n\nSurely size and 'n' should have the same signed-ness?\n\nAnd, in fact, shouldn't they actually be size_t, rather than 'int', \nsince xread is defined as:\n\nstatic inline ssize_t xread(int fd, void *buf, size_t len)\n{\n         ssize_t nr;\n         while (1) {\n                 nr = read(fd, buf, len);\n                 if ((nr < 0) && (errno == EAGAIN || errno == EINTR))\n                         continue;\n                 return nr;\n         }\n}\n\nAnd finally, 'ret' in safe_read should be a 'ssize_t', not an int, right?\n\nOr is it just a case that we don't really care, since we control the \nranges of the values, and the underlying types are int anyway? Patches \nto follow if I get an indication that anyone cares, otherwise I'd be \nposting my question to a C newbies group. ;-)\n\nRogan\n"},{"id":"38037","messageId":"20070326164755.GH13247@spearce.org","threadId":"7412","inReplyTo":"4607D4F6.80703@dawes.za.net","subject":"Re: Type mismatches in safe_read and friends?","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-03-26T16:47:55Z","receivedAt":"2007-03-26T16:47:55Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Rogan Dawes <lists@dawes.za.net> wrote:\n> I'm starting to learn a little C, and I figured I'd learn from the \n> masters ;-) I needed to read in some data from the network, and I \n> figured the safe_* calls would be a good example of how to do it correctly.\n...\n> static void safe_read(int fd, void *buffer, unsigned size)\n> {\n>         int n = 0;\n> \n>         while (n < size) {\n>                 int ret = xread(fd, (char *) buffer + n, size - n);\n...\n> Surely size and 'n' should have the same signed-ness?\n\nGah.  Yes.  And ret should be ssize_t.\n \n> And, in fact, shouldn't they actually be size_t, rather than 'int', \n> since xread is defined as:\n\nYes.\n\n> static inline ssize_t xread(int fd, void *buf, size_t len)\n...\n> And finally, 'ret' in safe_read should be a 'ssize_t', not an int, right?\n\nOh, I see you noticed that too.  ;-)\n \n> Or is it just a case that we don't really care, since we control the \n> ranges of the values, and the underlying types are int anyway? Patches \n> to follow if I get an indication that anyone cares, otherwise I'd be \n> posting my question to a C newbies group. ;-)\n\nIt is sort of a case we don't care.  These probably should be fixed.\nA patch would be nice.  You want to learn C...  ;-)\n\nWe currently assume that sizeof(unsigned) == sizeof(int) == 4,\nand that nobody is crazy enough to call this functions with values\nover ~2,000,000,000 so we don't practically have signed/unsigned\nissues here.  Right now anyway.  But it shouldn't be like this.\n\nSo size_t/ssize_t are the right types.\n\n\nThe one that cracks me up is what moron declared read(2) to take\nsize_t as the input argument and ssize_t as the return value.\nSo I can pass in a value that if successfully read by the kernel\nwill actually be < 0 upon return, making my code think the read\ncall failed - but it didn't.  Riiiiiiiiight.\n\nxread was just following that standard, broken model.\n\n-- \nShawn.\n"},{"id":"38133","messageId":"4608F31B.8070204@dawes.za.net","threadId":"7412","inReplyTo":"20070326164755.GH13247@spearce.org","subject":"Re: Type mismatches in safe_read and friends?","fromName":"Rogan Dawes","fromEmail":"lists@dawes.za.net","sentAt":"2007-03-27T10:34:03Z","receivedAt":"2007-03-27T10:34:03Z","isPatch":false,"sender":{"key":"lists@dawes.za.net","avatar":null},"body":"Shawn O. Pearce wrote:\n>  \n>> Or is it just a case that we don't really care, since we control the \n>> ranges of the values, and the underlying types are int anyway? Patches \n>> to follow if I get an indication that anyone cares, otherwise I'd be \n>> posting my question to a C newbies group. ;-)\n> \n> It is sort of a case we don't care.  These probably should be fixed.\n> A patch would be nice.  You want to learn C...  ;-)\n> \n\nOk. So I did a little patch, but I'm not sure whether I solved anything. \nNow we have an implicit cast from size_t to ssize_t in packet_read_line.\n\nI guess this echoes your comment about requesting a large size_t, and \ngetting a ssize_t result back. I suppose in theory we should be refusing \nto handle a length greater than that which would fit into a ssize_t? Or \nsimply making sure to always return data smaller than max(ssize_t)?\n\nPatch to follow.\n\nRogan\n"}]}