5 ms·

You may find the code for the current implementation in their CVS: http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/file/?sortby=date&only_with_tag=HEAD htt

by TheDong 12y ago

You may find the code for the current implementation in their CVS: http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/file/?sortby=date&only_with_tag=HEAD http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/file/?s... You can also find a git mirror of the referenced commit at: https://bitbucket.org/braindamaged/openbsd-src/commits/c421de15c47394de1ea2478edc0aa2c941aab2f8#chg-usr.bin/file/file.c https://bitbucket.org/braindamaged/openbsd-src/commits/c421d...

userbinator 12y ago
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/file/file.c?rev=1.32&content-type=text/x-cvsweb-markup&sortby=date&only_with_tag=HEAD http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/file/fi... I'm not so familiar with FreeBSD coding standards and design practices but it looks like that could still be refactored even more by getting rid of all the extra memory allocations (and deallocations), making future changes less likely to introduce bugs. For example it's allocating one structure for each file given as input, and making copies of all the paths... which I'm pretty sure isn't necessary as file tests each of its input files independently; the code misleadingly implies, however, that they are somehow dependent. The old version was better in this respect. That said, compared to the much more complex old version, the rest of it is an improvement.
- rjcz 12y ago
  OpenBSD != FreeBSD This[0] is a good place to start. Other than than, just read the code. [0] http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man9/style.9?query=style http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man9/...
  - userbinator 12y ago
    Getting that wrong shows how much I work with the different variants of * BSD... That's a syntactic style guide; I was referring to higher-level issues at the design level, like "avoid dynamic allocation unless absolutely necessary", which are between syntax and the general very-high-level "follow the UNIX philosophy" type of guidelines. To answer the sibling comments about preparing a patch and sending it in: this was just an observation I had from a brief glance at the code; I don't have the resources to delve into the research and find out if this is actually something they would want to change, if there is a reason behind doing it this way, or only the norm for the project (e.g. the Java community largely has a very different notion of what it means to "simplify" code) - so if someone wants to take these ideas and follow through, they are more than welcome to.
- stsp 12y ago
  Please take the time to send diffs to tech@ instead of writing code review comments on hackernews which will most likely not be read by relevent people and therefore have no effect whatsoever. Thanks.
  - musername 12y ago
    Is I'm pretty sure yet enough to warrant a report?
    - iwwr 12y ago
      There's nothing stopping you from submitting a patch for a small sample of the issues and a friendly question on their mailing list.
cnst 12y ago
OpenGrok on BSD Cross Reference is probably more useful in looking at it. http://BXR.SU/OpenBSD/usr.bin/file/file.c http://BXR.SU/OpenBSD/usr.bin/file/file.c.
- erikano 12y ago
  Never heard of that website, seems useful. Unless I am missing something, however, it seems that while the OpenBSD, NetBSD and DragonFlyBSD sources are up to date, the FreeBSD sources are out of date from 09-Dec-2013.
- przemoc 12y ago
  Thanks for mentioning Super User's BSD Cross Reference (bxr.su)! Apparently there are many more OpenGrok installations nowadays than it used to be 5 years ago. https://github.com/OpenGrok/OpenGrok/wiki/OpenGrok-installations https://github.com/OpenGrok/OpenGrok/wiki/OpenGrok-installat... EDIT: Not all of them are working, though.

by TheDong 12y ago

userbinator 12y ago

http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/file/file.c?rev=1.32&content-type=text/x-cvsweb-markup&sortby=date&only_with_tag=HEAD http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/file/fi... I'm not so familiar with FreeBSD coding standards and design practices but it looks like that could still be refactored even more by getting rid of all the extra memory allocations (and deallocations), making future changes less likely to introduce bugs. For example it's allocating one structure for each file given as input, and making copies of all the paths... which I'm pretty sure isn't necessary as file tests each of its input files independently; the code misleadingly implies, however, that they are somehow dependent. The old version was better in this respect. That said, compared to the much more complex old version, the rest of it is an improvement.

rjcz 12y ago
OpenBSD != FreeBSD This[0] is a good place to start. Other than than, just read the code. [0] http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man9/style.9?query=style http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man9/...
- userbinator 12y ago
  Getting that wrong shows how much I work with the different variants of * BSD... That's a syntactic style guide; I was referring to higher-level issues at the design level, like "avoid dynamic allocation unless absolutely necessary", which are between syntax and the general very-high-level "follow the UNIX philosophy" type of guidelines. To answer the sibling comments about preparing a patch and sending it in: this was just an observation I had from a brief glance at the code; I don't have the resources to delve into the research and find out if this is actually something they would want to change, if there is a reason behind doing it this way, or only the norm for the project (e.g. the Java community largely has a very different notion of what it means to "simplify" code) - so if someone wants to take these ideas and follow through, they are more than welcome to.
stsp 12y ago
Please take the time to send diffs to tech@ instead of writing code review comments on hackernews which will most likely not be read by relevent people and therefore have no effect whatsoever. Thanks.
- musername 12y ago
  Is I'm pretty sure yet enough to warrant a report?
  - iwwr 12y ago
    There's nothing stopping you from submitting a patch for a small sample of the issues and a friendly question on their mailing list.

cnst 12y ago

OpenGrok on BSD Cross Reference is probably more useful in looking at it. http://BXR.SU/OpenBSD/usr.bin/file/file.c http://BXR.SU/OpenBSD/usr.bin/file/file.c.

erikano 12y ago
Never heard of that website, seems useful. Unless I am missing something, however, it seems that while the OpenBSD, NetBSD and DragonFlyBSD sources are up to date, the FreeBSD sources are out of date from 09-Dec-2013.
przemoc 12y ago
Thanks for mentioning Super User's BSD Cross Reference (bxr.su)! Apparently there are many more OpenGrok installations nowadays than it used to be 5 years ago. https://github.com/OpenGrok/OpenGrok/wiki/OpenGrok-installations https://github.com/OpenGrok/OpenGrok/wiki/OpenGrok-installat... EDIT: Not all of them are working, though.