

Show HN: c - Jonovono
https://github.com/Jonovono/C

======
leif
Comments for your C learning:

    
    
        #include <stdbool.h>
    

is better than

    
    
        typedef int bool;
        #define true 1
        #define false 0
    

Always, always, surround your if and else clauses with brackets. There are
plenty of ways leaving them naked will screw you over. This goes for any
language.

This is terrifying, and broken:

    
    
        char * s = malloc(snprintf(NULL, 0, "%s/%s", cwd, argv[1]) + 1);
        sprintf(s, "%s/%s", cwd, argv[1]);
    

Passing NULL to snprintf, first of all, should be a huge red flag for you. If
it's not, train yourself to recognize it. I realize what you're trying to do,
but it relies on non-portable behavior. In particular, from the manual:

    
    
        The glibc implementation of the functions snprintf() and vsnprintf() conforms
        to the C99 standard, that is, behaves as described above, since glibc version
        2.1.  Until glibc 2.0.6 they would return -1 when the output was truncated.
    

So until glibc 2.0.6, your program would be allocating 0 and then sprintfing
into it. Always be careful when you read the manual, to read everything. And
even then, program defensively, and train yourself to watch for errors like
this one. In this case, passing NULL as the destination argument was an
immediate no-no.

In this function, you really should be checking errno (for ENOENT). stat(2)
can fail for all sorts of reasons, one of them is that the entry is not there.

    
    
        bool dirOrFileExists(const char dir[]) {
    	struct stat st;
    	if (stat(dir, &st) == 0) {
    		return true;
    	}
    	else {
    		return false;
    	}
        }
    

Same for this loop, check errno to make sure the function's failing for the
reason you think it should.

    
    
        while((entry = readdir(mydir)))
    

Here, you have something a little messy:

    
    
        int fileOrDirectory(const char path[]) {
    	struct stat s;
    
    	if (stat(path, &s) == 0) {
    		if (s.st_mode & S_IFDIR) {
    			// Is a directory
    			return 0;
    		} else if (s.st_mode & S_IFREG) {
    			// Is a file
    			return 1;
    		} else {
    			// Something else
    			return -1;
    		}
    	}
    	else {
    		// Error
    		printf("Error occurred\n");
    	}
        }
    

First of all, returning values only you know about is bound for destruction.
You should define an enum if you want a function to return specific states.
Better yet, just do these checks in the function that would normally call
this. It's not so bad usually, and you're probably calling stat(2) more than
you should. It's a syscall, and those can be expensive. But more importantly,
you have a branch where all you do is print something (to stdout, not stderr
as you should), and then just return nothing! If you hit an error state, you
need to either propagate that error up and handle it in the caller, or crash
immediately. Don't leave your program in a garbage state and keep going. A
compiler should have warned you about this (not having a return value in one
branch). Turn on -Werror and at least -Wall. And fix your warnings, don't try
to suppress them.

You should use fscanf(3) instead of this:

    
    
        while (1) {
    	ch = fgetc(fp);
    	newch[i] = ch;
    	if (ch == EOF) {
    		newch[i] = '\0';
    		break;
    	}
    	i++;
        }
    

That's all for now, I gotta go. Good luck!

~~~
haberman
> So until glibc 2.0.6, your program would be allocating 0 and then sprintfing
> into it.

glibc 2.0.6 was released in 1997. I do not think it is reasonable to ask that
applications accommodate bugs in libc that have been fixed for 15 years. If
someone was actually using a system that old, their system will have so many
unpatched vulnerabilities that this will be the least of their problems. And
it's highly unlikely that every such bug is documented in the manpages anyway.

~~~
lubutu
glibc is not the only libc in existence. It _is_ reasonable to ask that
applications support systems other than those developed by GNU.

~~~
chc
Supporting implementations that do not conform to the language standard might
be very important to you or it might not. I hardly think it's a moral
imperative.

------
redbad

        And those on Mac should be able to use:
        
        $ brew install c
    

Wow, uh, didn't it strike you that this might be a bad idea?

~~~
sharth
As far as I can tell, this program is not included in Homebrew.

~~~
Jonovono
hmm. It should be. You may just have to run brew update? It wasn't added very
long ago so you may not see it if brew has not been updated. If it still
doesn't work then not sure.

~~~
mjschultz
Are you sure you committed it to the homebrew repository on github? Your
github page doesn't show any activity that indicates you've forked the
homebrew repo or created a pull request in the past 14 days (since you made
the "c" repo).

Even so, based on the general rules for homebrew inclusion they would reject
the formula because:

1\. I don't think they like names that have the potential to conflict with
others ("c" is in that category). The shortest name I see in their repo is two
chars.

2\. Typically they only want widely used formulas ("We generally frown on
authors submitting their own work unless it is suitably notable." [1]).

[1] <https://github.com/mxcl/homebrew/wiki/Acceptable-Formula>

~~~
Jonovono
hmm interesting. I just followed the instructions on
<http://mxcl.github.com/homebrew/> and when I run brew install c in my
terminal it says "Error: c-1.0.0 already installed". But if it is not a
formula they would want then I guess I should not be too worried.

~~~
mjschultz
That just means you have a local formula that knows how to build and install
c, but you have not shared it with the community so no one else will have
access to the formula. You'd need to fork, push to your fork, and create a
pull request to the main homebrew repo to try for inclusion.

Also, it is only my hunch that they wouldn't accept the formula--you can still
try. I'd still rename the project to be more descriptive (dir-c?) as there are
only two homebrew formulae that have single char names (R, which has been
around since 1993 and Z, which I'm surprised got included).

~~~
Jonovono
Ahhh, makes sense. Thanks. I'll fix it up and maybe try submitting it.

------
davyjones
Terrible, terrible name.

~~~
veyron
i agree that no program should have 1 character name, but what do you
currently alias 'c' to?

~~~
suraj
clear screen

~~~
lubutu
There is no reason to use clear during a terminal session. ^L (Ctrl-L) does
this for you in one key press, without needing to clear your input or fill
your history.

~~~
acuozzo
> There is no reason to use clear during a terminal session. ^L (Ctrl-L) does
> this for you in one key press, without needing to clear your input or fill
> your history.

Isn't this based on the assumption that one's shell explicitly supports ^L or
has editline/readline support?

------
ajanuary

        if (getcwd(cwd, sizeof(cwd)) != NULL)
            ;
        else
            perror("getcwd() error");
    

Is that common? What's the advantages over the below?

    
    
        if (... == NULL) {
            perror(...);
        }

~~~
tptacek
No, it's not common. This is not idiomatic C code (not that that's an
intrinsic problem).

------
zhov
I would rename it to something else and just alias it to 'c'.

------
markdrago
It may be more appropriate to store the comments in extended attributes rather
than in dotfiles: <http://en.wikipedia.org/wiki/Extended_file_attributes>

------
jcromartie
Hidden files are handy for some things, but a README.txt file would be
apparent to anybody who comes across the directory, and not just someone who
knows that the hidden directory is/may be there.

------
axitanull
Googling topics on "C" has been tough for me for the past 15 years.

Bonus point for me trying to filter out C++.

------
zhov
I'd also like to point out that the Twitter account you link to for contact
looks extremely inactive.

~~~
Jonovono
It is. But I plan to start using it. I just started using twitter and only
really plan to use it to share projects I work on or communicate with others!

------
mfonda
I love this post and the exchange of comments going on here. The OP is just
starting to learn C, and to help learn it got started on a project that they
found useful, which I think is one of the best ways to learn programming.

The community then responded by posting helpful feedback, much of which
demonstrates the idiomatic way of doing things, which is often one of the most
difficult things to learn when picking up a new programming language. Other
helpful advice is posted on build options and debug tools.

This kind of constructive and useful feedback is great to see.

------
mccoyst
Why did you decide to use autoconf?

------
azelfrath
As a sub-novice C/++ programmer myself, it's awesome to see a program like
this being discussed. The source code is relatively short, it's a simple
enough implementation, and all the feedback in the comments here is teaching
me things I wouldn't necessarily be exposed to in a book.

~~~
Jonovono
Yeah! I am happy with the feedback. I just made this because I thought it
would be useful for me and to help learn C since I just got K&R book. Also, I
know people are always looking for open source projects to submit to so this
would be perfect to try out as there are most likely (as pointed out in the
comments) many things that can be improved!

------
gouranga
Great idea - I think 'c' should be a shell alias though.

Tagging support would be cool.

~~~
Jonovono
Thanks!

And yeah, didn't think of that. It would be neat to be able to tag files or
directories and then execute a command to see all the directories with that
tag and then enter that directory easily. Maybe i'll try adding that to the
next version. Can you think of anything else that would be useful?

------
ga2arch
Wouldn't it be better to use Extended file attributes ?
<https://gist.github.com/2695264> (actually my first attempt to a bash script)

~~~
brimpa
I'd prefer to have all the comments located in a single location (e.g.
~/.comments or ~/Dropbox/comments-<hostname>) rather than litter hidden files
all over the place because I could (1) easily back them up, (2) share them
(even across different platforms), and (3) allow multiple users to set local
and global (shared) comments.

EDIT

Because I was bored: <https://gist.github.com/2697095>

------
zephyrfalcon
4DOS used to have this. It was especially useful back in the day with the 8.3
DOS filenames, which often weren't very descriptive. It's nice to see this
feature return.

------
swordswinger12
This is really cool! Thanks for sharing.

~~~
Jonovono
Thanks! Glad you like it.

