Thread: Help with fgets - Android recovery

  1. #1
    Registered User
    Join Date
    May 2012
    Posts
    5

    Help with fgets - Android recovery

    I'm working on a custom recovery for Android written in c. I'm trying to add a feature to return the current ROM to the user by using fgets to read the build.prop file available on every phone. The code below works on one ROM where ro.modversion appears at line 106, but doesn't work on a similar ROM where ro.modversion appears at line 109.

    Code:
    1. char* get_modversion()
    2. {
    3. char* result;
    4. ensure_root_path_mounted("SYSTEM:");
    5. FILE * vers = fopen("/system/build.prop", "r");
    6. if (vers == NULL)
    7. {
    8. return NULL;
    9. }
    10. char line[512];
    11. while(fgets(line, sizeof(line), vers) != NULL && fgets(line, sizeof(line), vers) != EOF) //read a line
    12. {
    13. if (strstr(line, "ro.modversion") != NULL)
    14. {
    15. char* strptr = strstr(line, "=") + 1;
    16. result = calloc(strlen(strptr) + 1, sizeof(char));
    17. strcpy(result, strptr);
    18. break;
    19. }
    20. }
    21. fclose(vers);
    22. ensure_root_path_unmounted("SYSTEM:");
    23. ui_print("Current ROM: %s\n", result);
    24. int LENGTH = strlen(result);
    25. ui_print("LENGTH: %d\n", LENGTH);
    26. return LENGTH;
    27. }
    On the first ROM build.prop it returns
    Current ROM: Decks_Reloaded
    LENGTH:15 (14 characters plus the new line character fgets adds).

    On the second ROM build.prop, it returns
    Current ROM:
    moving:
    LENGTH: 10


    Seems like it's returning something if the length is 10, but I don't know what. Also don't know what "moving:" means. I've tried lowering char line[] to 128 and increasing it up to 4096. I've tried changing calloc to malloc. I've tried changing sizeof(char) to sizeof(line) in result=. No matter what it works on the first but not the second. (wouldn't build when I changed it to malloc). Any help would be greatly appreciated.

  2. #2
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    A couple of comments

    while(fgets(line, sizeof(line), vers) != NULL && fgets(line, sizeof(line), vers) != EOF)
    The second part of this is completely pointless. fgets() does not return EOF
    Another reason it is pointless is that you're calling fgets() TWICE for each loop iteration, which probably means it only "works" when the line number is even rather than odd.

    while ( fgets(line, sizeof(line), vers) != NULL )
    is all you need.


    > char* result;
    It's a good idea to initialise ALL your pointers at the outset, just in case there is a path through the code which doesn't update the pointer.
    In this case, you don't find a match (see broken loop above) and you're pointing at garbage.

    > char* get_modversion()
    Make your mind up on the signature
    Your main return path returns an integer, not a pointer.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  3. #3
    Registered User
    Join Date
    May 2012
    Posts
    5
    Thank you for the input, Salem. I'll give those changes a try.

  4. #4
    Registered User
    Join Date
    May 2012
    Posts
    5
    Salem, you were right on with all of your comments. Thanks for your help. I now need to create some if/else statements because some ROMs use ro.build.description or ro.build.version instead of ro.modversion, but that seems doable now that it works on all build.props that use ro.modversion. Thanks again.

  5. #5
    Master Apprentice phantomotap's Avatar
    Join Date
    Jan 2008
    Posts
    5,108
    I'm not sure I want someone who doesn't know how to code trying to make a custom recovery for a probably expensive device.

    As a personal favor to Salem, would you mind practicing for the next three years before you try to make a recovery tool?

    Soma

  6. #6
    Registered User
    Join Date
    May 2012
    Posts
    5
    Gosh, why do people always have to be so harsh? I am learning and I've spent the past few months learning how to build this recovery from source. That's not exactly something a complete noob could figure out. If it helps you, I did order K&R and plan on learning more. I thought starting with fgets would be a pretty safe thing to add to an existing recovery I'm modifying. You know, that whole open source model? Salem helped me and I really appreciate that. I've helped a lot of people on forums myself, just not this one. This is the final code I ended up with and it works good.

    Code:
    void get_my_rom_info()
    {
      ui_print("\n");
      ui_print("Getting current ROM version...\n");
      char* result;
      ensure_root_path_mounted("SYSTEM:");
      FILE * vers = fopen("/system/build.prop", "r");
      if (vers == NULL) 
      {
        return NULL;
      }
    
    
      char line[512];
      while(fgets(line, sizeof(line), vers) != NULL) //read a line
      {
        if (strstr(line, "modversion") != NULL || strstr(line, "romversion") != NULL)
        {
          char* strptr = strstr(line, "=") + 1;
          result = calloc(strlen(strptr) + 1, sizeof(line));
          strcpy(result, strptr);
          break;  
        }
      }
      fclose(vers); 
      ensure_root_path_unmounted("SYSTEM:");
      ui_print("\nCurrent ROM: %s\n", result);
     
    }

  7. #7
    Registered User
    Join Date
    Dec 2011
    Posts
    795
    > and it works good.

    Code:
    void get_my_rom_info()
    /* ... */
        return NULL;
    Your return type is void. Why are you returning something??!

    Code:
    /* ... */
    result = calloc(strlen(strptr) + 1, sizeof(line));
    /* ... */
    Why aren't you free()ing memory? And do you know how calloc works? Why would you need "512 * strlen(strptr) + 1" bytes? In fact, why do you even need a new variable? If you're not returning anything, why don't you just print out the result of the last strstr()?

  8. #8
    Registered User
    Join Date
    May 2012
    Posts
    5
    You can view the whole code base here. The main ui is setup in recovery.c. I put this option to "Get Current ROM Version" in the Other Menu and all of those options use extracommands.c. I tried to follow the syntax of the check_my_battery_level () option to create this choice. When I run it on my phone, it works exactly like I wanted it to... it returns "Current ROM: gingerbread_evo_deck1.3d" or the name of whatever other ROM I'm running.

    The lines in build.prop probably never exceed 128, so you're right it probably doesn't need to be 512. Thanks for your input. I'm learning C and can't say I completely understand everything about fgets or calloc, but I certainly read the man pages and am trying to learn.

    BTW, that return NULL is if the file build.prop doesn't exist (which is always would on an android phone, I believe). Right?
    Last edited by smelkus; 06-04-2012 at 02:27 PM.

  9. #9
    Registered User
    Join Date
    Oct 2008
    Location
    TX
    Posts
    2,059
    Quote Originally Posted by smelkus View Post
    BTW, that return NULL is if the file build.prop doesn't exist (which is always would on an android phone, I believe). Right?
    NULL ain't void, so change the get_my_rom_info() return type to be FILE*.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Are there any (C/C++) Android developers around?
    By phantomotap in forum Tech Board
    Replies: 0
    Last Post: 09-06-2011, 12:19 PM
  2. Android Developement without Java
    By manasij7479 in forum Linux Programming
    Replies: 1
    Last Post: 05-13-2011, 05:43 PM
  3. Video link between Android, VLC and Dream box aka DBox
    By Eman in forum General Discussions
    Replies: 0
    Last Post: 04-30-2011, 02:25 AM
  4. File Recovery Help
    By anirban in forum Tech Board
    Replies: 1
    Last Post: 06-06-2008, 02:35 PM