CEMETECH
Leading The Way To The Future
Login [Register]
Username:
Password:
Autologin:

Don't have an account? Register now to chat, post, use our tools, and much more.
Latest Headlines
» Cemetech Member Carves Intricate Wooden TI-83+
» HP Prime Multitouch Graphing Calculator: The Scoop
» More Details on the Casio fx-CP400 Touchscreen Calculator
» Editorial: On Choosing and Shelving Projects
Online Users
There are 117 users online: 1 member, 91 guests and 25 bots.
Members: None.
Bots: VoilaBot (1), MSN/Bing (2), Magpie Crawler (3), Googlebot (18), MSN/Bing (1).
RSS & Social Media
[t] Follow @cemetech
[t] 'Like' Cemetech
[RSS] News Headlines
[RSS] Latest Posts
SAX
You must log in to view the SAX chat widget
Anyone want to check some C code? :)
» >
    »
» View previous topic :: View next topic  
Author Message
Jim Bauwens


New Member


Joined: 05 Dec 2006
Posts: 96
Location: Belgium
Posted: 20 Jan 2012 09:22:22 am    Post subject: Anyone want to check some C code? :)
I'm busy on modifying Luna (a program that generates documents for the TI-Nspire).

I don't have that much experience with C, but I managed to do what I wanted. But I'm just wondering if I'm doing everything the right way.
It's the first time I use malloc and co, so you never know Smile

Here is my code:

Code:

   //Extension by Jim Bauwens
   if (argc>3){
      FILE *FP;
      int ifile;
      fpos_t pos;
      char *buf ;
      
      for (ifile=3; ifile<argc; ifile++){
         if ((FP=fopen(argv[ifile], "r")) == NULL) {
            printf("Couldn't open %s for reading, ignoring it\n", argv[ifile]);
            break;
         }

         if (fseek(FP, 0, SEEK_END)==-1){
            printf("Couldn't jump to end of file\n");
            break;
         }

         fgetpos(FP, &pos);
         rewind(FP);
         
         buf = (char *) malloc(pos.__pos);
         if (buf == NULL) {
            printf("Could not allocate memory!\n");
              break;
           }
           
           fread(buf, sizeof(char), pos.__pos-1, FP);
         
         if (ferror(FP)){
            printf("Error reading %s!\n", argv[ifile]);
            clearerr(FP);
            break;
         }
         
         if (zipOpenNewFileInZip2(zf, argv[ifile], &zi, NULL, 0, NULL, 0, NULL, 0xD, 0, 1) != ZIP_OK) {
            printf("can't zip %s", argv[ifile]);
            goto close_quit;
         }         
         if (zipWriteInFileInZip(zf, buf, pos.__pos) != ZIP_OK) {
            puts("can't add file to zip file");
            goto close_quit;
         }         
         if (zipCloseFileInZipRaw(zf, pos.__pos, 0) != ZIP_OK) {
            puts("can't close file in zip file");
            goto unlink_quit;
         }   
               
         fclose(FP);
         free(buf);      
      }
   }
   //End Extension


Any bad errors in that?
TIA Smile
_________________
TI-Nspire projects of me:
nCreator | PCspire | Klondike Lua | LogoMagic | EEPro for the TI-Nspire | Pegs | General math definitions
Back to top
Ashbad


... I think redheaded girls are kind of cool


Joined: 01 Dec 2010
Posts: 2418
Location: Stomp Stomp Stomp, The Idiot Convention
Posted: 20 Jan 2012 09:28:32 am    Post subject:
I'm assuming you need those gotos for jumping around? Otherwise, why not use functions? It's a bit cleaner. Other than that, looks fine.
_________________
-Ashbad
Back to top
Jim Bauwens


New Member


Joined: 05 Dec 2006
Posts: 96
Location: Belgium
Posted: 20 Jan 2012 09:53:25 am    Post subject:
The labels I'm jumping to are part of the original program, and I'm not going to start to rewrite that Smile

So, my malloc stuff is good?
Very Happy
_________________
TI-Nspire projects of me:
nCreator | PCspire | Klondike Lua | LogoMagic | EEPro for the TI-Nspire | Pegs | General math definitions
Back to top
KermMartian


Site Admin


Joined: 14 Mar 2005
Posts: 55760
Location: Earth, Sol, Milky Way
Posted: 20 Jan 2012 10:15:17 am    Post subject:
The malloc() stuff is fine. There are a few things that you could express a bit more cleanly by combining statements, and a few bits of tabbing that are aligned counter-intuitively, but the code is solid. Smile
_________________
Back to top
Tari


Systems Integrator


Joined: 03 Jul 2006
Posts: 2112
Location: Always-winter, Michigan
Posted: 20 Jan 2012 10:17:47 am    Post subject: Re: Anyone want to check some C code? :)

Code:
    fpos_t pos;
    char *buf ;

    fgetpos(FP, &pos);
    buf = (char *) malloc(pos.__pos);

man fpos_t wrote:
The content of a fpos_t is not meant to be read directly, but only to use its reference as an argument in a call to fsetpos.

Use ftell.

Usually, if something's name has one or more underscores leading, you shouldn't use it without good reason.
_________________


Ask questions the smart way · タリ
Back to top
christop


Power User


Joined: 09 Mar 2011
Posts: 385
Location: Arizona, USA
Posted: 20 Jan 2012 02:31:08 pm    Post subject: Re: Anyone want to check some C code? :)
I don't see any big issues besides what others have pointed out, but there's the little issue of writing error messages to stdout. Error messages should be written to stderr; after all, that's why stderr exists (stderr and stdout both typically output to the terminal, but they can be redirected separately, which is pretty useful). If the existing code writes error messages to stdout already, then unless you plan to change the rest of the code to use stderr, you should leave your extension as-is because consistency is pretty important too.

Regarding goto: it's one of the best and simplest ways to implement exception/error handling in C. You can easily make error handlers fall through to "unwind" or "undo" actions you've done before the error happened. For a good example of using goto for error handling, see the Linux kernel (http://kerneltrap.org/node/553/2131 is a good discussion of goto in Linux).
_________________
Christopher Williams
Back to top
Jim Bauwens


New Member


Joined: 05 Dec 2006
Posts: 96
Location: Belgium
Posted: 20 Jan 2012 02:49:57 pm    Post subject:
KermMartian wrote:
There are a few things that you could express a bit more cleanly by combining statements

Yeah, I know. But I first want to get everything working before I group the file reading and zipping part Smile

KermMartian wrote:

and a few bits of tabbing that are aligned counter-intuitively, but the code is solid. Smile

That was because of a copy/paste fail + tab/space mixing at one spot.
Looks good in my editor Razz

Tari wrote:

Use ftell.

Usually, if something's name has one or more underscores leading, you shouldn't use it without good reason.

Thanks Smile
I originally was going to use ftell, but I don't know why I used this in the end. I found the __pos thing online, didn't know it was not supposed to be used. I'll remember that, thanks Smile


Quote:
I don't see any big issues besides what others have pointed out, but there's the little issue of writing error messages to stdout. Error messages should be written to stderr; after all, that's why stderr exists (stderr and stdout both typically output to the terminal, but they can be redirected separately, which is pretty useful). If the existing code writes error messages to stdout already, then unless you plan to change the rest of the code to use stderr, you should leave your extension as-is because consistency is pretty important too.

Good point Smile
The original code uses puts for its errors, so the same style as stdout (I think).
I could easily change the other code, but I don't know if its best if I wish to keep up with updates.

Thanks everyone for your interesting and smart answers Smile
_________________
TI-Nspire projects of me:
nCreator | PCspire | Klondike Lua | LogoMagic | EEPro for the TI-Nspire | Pegs | General math definitions
Back to top
Display posts from previous:   
Register to Join the Conversation
Have your own thoughts to add to this or any other topic? Want to ask a question, offer a suggestion, share your own programs and projects, upload a file to the file archives, get help with calculator and computer programming, or simply chat with like-minded coders and tech and calculator enthusiasts via the site-wide AJAX SAX widget? Registration for a free Cemetech account only takes a minute.

» Go to Registration page
Anyone want to check some C code? :)
    »
» View previous topic :: View next topic  
Page 1 of 1 » All times are GMT - 5 Hours

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum

© Copyright 2000-2013 Cemetech & Kerm Martian :: Page Execution Time: 0.036210 seconds.