update.c using appcast.xml instead of LATEST

Archive of historical development discussions
Discussions / Development has moved to GitHub
Forum rules
*******************************
Please be aware we are now using GitHub for issue tracking and feature requests.
- This section of the forum is now closed to new topics.

*******************************
Post Reply
User avatar
s55
HandBrake Team
Posts: 10350
Joined: Sun Dec 24, 2006 1:05 pm

update.c using appcast.xml instead of LATEST

Post by s55 »

Code: Select all

Index: update.c
===================================================================
--- update.c	(revision 1402)
+++ update.c	(working copy)
@@ -7,7 +7,7 @@
 #include "hb.h"
 
 #define HB_URL   "handbrake.fr"
-#define HB_QUERY "GET /LATEST HTTP/1.0\r\nHost: " HB_URL "\r\n\r\n"
+#define HB_QUERY "GET /appcast.xml HTTP/1.0\r\nHost: " HB_URL "\r\n\r\n"
 
 typedef struct
 {
@@ -34,7 +34,7 @@
 
     hb_net_t * net;
     int        ret;
-    char       buf[1024];
+    char       buf[512];
     char     * cur, * end, * p;
     int        size;
     int        stable, unstable;
@@ -53,7 +53,7 @@
     }
 
     size = 0;
-    memset( buf, 0, 1024 );
+    memset( buf, 0, 512 );
     for( ;; )
     {
         ret = hb_net_recv( net, &buf[size], sizeof( buf ) - size );
@@ -92,22 +92,33 @@
     {
         goto error;
     }
-
-    stable = strtol( cur, &p, 10 );
-    if( cur == p )
-    {
-        goto error;
-    }
-    cur = p + 1;
-    memset( stable_str, 0, sizeof( stable_str ) );
-    for( i = 0;
-         i < sizeof( stable_str ) - 1 && cur < end && *cur != '\n';
-         i++, cur++ )
-    {
-        stable_str[i] = *cur;
-    }
-
+	
+	/*
+	 * Shift the cur memory pointer along 156 places to the start of the build number
+	 * conver the next 10 characters from a string to an integer.
+	 */
+    cur += 156;
+	stable = strtol( cur, &cur, 10 );
+	printf("Stable Build: %i \n", stable);
+	
+	/*
+	 * Shift the cur memory pointer along 9 places to the start of the version number string.
+	 * There can be up to 6 characters for a version string
+	 */
+	cur += 9;
+	
+	for( i = 0;   i < sizeof( stable_str ) - 1 && cur < end && *cur != '"'; i++, cur++ )
+	{
+		stable_str[i] = *cur;
+	}
+	printf("Stable Version: %s \n", stable_str);
+	
     hb_log( "latest stable: %s, build %d", stable_str, stable );
+	
+	
+	/* -- Return to normal programming -- */
+	
+	
 
     cur++;
     if( cur >= end )
Thats my patch so far.

Basically, what i've done is add a new tag to the appcast.xml file

<?xml version="1.0" encoding="utf-8"?>
<rss version="2.0" xmlns:sparkle="http://www.andymatuschak.org/xml-namespaces/sparkle">
<channel>
<cli>version="2009021900" short="0.9.4"</cli>
<title>HandBrake Appcast</title>

...

Then, the patch finds the version and short numbers and places them in variables "stable" and "stable_str" which were being used previously. Currently, the position of the data for both vars is hard coded. The version number (x.x.x) can change length no problem, so we can do (0.10.0) if we want. There is a for loop that handles that ok.
There are 2 printf statements that correctly print out the version numbers currently. These can be removed later.

I haven't yet touched the code for the unstable stuff. Most of that can probably come out unless we want to add yet another tag in the xml file for an unstable version.

Thoughts so far?

Someone please slap me next time I decide to do something without first knowing the language it was written in.
dynaflash
Veteran User
Posts: 3820
Joined: Thu Nov 02, 2006 8:19 pm

Re: update.c using appcast.xml instead of LATEST

Post by dynaflash »

sr55 wrote:Someone please slap me next time I decide to do something without first knowing the language it was written in.
Consider yourself slapped. :)
jbrjake
Veteran User
Posts: 4805
Joined: Wed Dec 13, 2006 1:38 am

Re: update.c using appcast.xml instead of LATEST

Post by jbrjake »

Looks good.
sr55 wrote:I haven't yet touched the code for the unstable stuff. Most of that can probably come out unless we want to add yet another tag in the xml file for an unstable version.
I'd like to keep it, please. If the HB_BUILD of the binary doing the update check ends in 01 instead of 00, it should work off the unstable info instead of the stable.
User avatar
s55
HandBrake Team
Posts: 10350
Joined: Sun Dec 24, 2006 1:05 pm

Re: update.c using appcast.xml instead of LATEST

Post by s55 »

OK, I'll sort it out tomorrow with a revised patch
User avatar
s55
HandBrake Team
Posts: 10350
Joined: Sun Dec 24, 2006 1:05 pm

Re: update.c using appcast.xml instead of LATEST

Post by s55 »

Code: Select all

Index: update.c
===================================================================
--- update.c	(revision 1402)
+++ update.c	(working copy)
@@ -7,7 +7,7 @@
 #include "hb.h"
 
 #define HB_URL   "handbrake.fr"
-#define HB_QUERY "GET /LATEST HTTP/1.0\r\nHost: " HB_URL "\r\n\r\n"
+#define HB_QUERY "GET /appcast.xml HTTP/1.0\r\nHost: " HB_URL "\r\n\r\n"
 
 typedef struct
 {
@@ -92,45 +92,96 @@
     {
         goto error;
     }
+	
+	
+	
+	
+	
+	
+	/*
+	 * Find the <cli-stable> tag
+	 * Scan though each character of the buffer until we find that the first 4 characters of "cur" are "<cli"
+	 */
 
-    stable = strtol( cur, &p, 10 );
-    if( cur == p )
-    {
-        goto error;
-    }
-    cur = p + 1;
-    memset( stable_str, 0, sizeof( stable_str ) );
-    for( i = 0;
-         i < sizeof( stable_str ) - 1 && cur < end && *cur != '\n';
-         i++, cur++ )
-    {
-        stable_str[i] = *cur;
-    }
-
+     for( ; &cur[3] < end; cur++ )
+     {
+        if( cur[0] == '<' && cur[1] == 'c' && cur[2] == 'l' && cur[3] == 'i' )
+         {
+            cur += 1;
+            break;
+         }
+     }
+	
+	/*
+	 * Ok, The above code didn't position cur, it only found <cli so we need to shift cur along 11 places.
+	 * After which, the next 10 characters are the build number
+	 */
+    cur += 11;
+	stable = strtol( cur, &cur, 10 );
+	printf("Stable Build: %i \n", stable);
+	
+	/*
+	 * The Version number is 2 places after the build, so shift cur, 2 places.
+	 * Get all the characters in cur until the point where " is found.
+	 */
+	cur += 2;
+	for( i = 0;   i < sizeof( stable_str ) - 1 && cur < end && *cur != '"'; i++, cur++ )
+	{
+		stable_str[i] = *cur;
+	}
+	printf("Stable Version: %s \n", stable_str);
+	
     hb_log( "latest stable: %s, build %d", stable_str, stable );
+	
 
-    cur++;
-    if( cur >= end )
-    {
-        goto error;
-    }
 
-    unstable = strtol( cur, &p, 10 );
-    if( cur == p )
-    {
-        goto error;
-    }
-    cur = p + 1;
-    memset( unstable_str, 0, sizeof( unstable_str ) );
-    for( i = 0;
-         i < sizeof( unstable_str ) - 1 && cur < end && *cur != '\n';
-         i++, cur++ )
-    {
-        unstable_str[i] = *cur;
-    }
 
+
+
+	/*
+	 * Find the <cli-unstable> tag
+	 * Scan though each character of the buffer until we find that the first 4 characters of "cur" are "<cli"
+	 */
+
+     for( ; &cur[3] < end; cur++ )
+     {
+        if( cur[0] == '<' && cur[1] == 'c' && cur[2] == 'l' && cur[3] == 'i' )
+         {
+            cur += 1;
+            break;
+         }
+     }
+	 
+	/*
+	 * Now we need to handle the unstable build information
+	 * Unstable build number is 29 Characters after the last position used.
+	 */
+	 
+	 cur += 13;     
+	 unstable = strtol( cur, &p, 10 );
+	 printf("Unstable Build: %i \n", unstable);
+	
+	/*
+	 * Now we need to get the unstable version number.
+	 * First move the cur pointer 12 places.
+	 * Then iterate over cur until " is found. Thats the end of the version number.
+	 */
+	cur += 12;
+	for( i = 0;   i < sizeof( unstable_str ) - 1 && cur < end && *cur != '"'; i++, cur++ )
+	{
+		unstable_str[i] = *cur;
+	}
+	printf("Unstable Version: %s \n", unstable_str);
+	printf("\n\n");
+	
+
     hb_log( "latest unstable: %s, build %d", unstable_str, unstable );
 
+
+	/*
+	 * Handle the update checking as normal.
+	 * This code is unchanged.
+	 */
     if( HB_BUILD % 100 )
     {
         /* We are runnning an unstable build */
Example output. I've left a few debug printf statements in for the moment.
Scott@q6 ~/new
$ ./HandBrakeCLI -u
Stable Build: 2009021900
Stable Version: 0.9.4
Unstable Build: 2009021901
Unstable Version: 0.9.3


HandBrake svn1402M (2008041101) - http://handbrake.m0k.org/
You are using an old version of HandBrake.
Latest is 0.9.3 (build 2009021901).
Ok, That's version 2 of the patch.
It now works a bit better. Instead of relying on the CLI tags being in an exact position, it now searches through the file for the <cli which, from there, it can find the version numbers with exact positioning.
It now also handles unstable versions.
I have 2 tags in the appcast.xml file now. under the <title> tags.
<cli-stable>2009021900 "0.9.4" </cli-stable>
<cli-unstable>2009021901 "0.9.3" </cli-unstable>
The only odd thing is, "HandBrake svn1402M (2008041101)" Is that supposed to have a version number in front of it? Not sure if this is something i've broken myself??

Can someone familiar with C look over the code and make sure I haven't screwed anything up please? I'm a wee bit worried about the lines

Code: Select all

memset( unstable_str, 0, sizeof( unstable_str ) );
that I removed. Not really sure what they do, and even if they are in fact needed any more?
eddyg
Veteran User
Posts: 798
Joined: Mon Apr 23, 2007 3:34 am

Re: update.c using appcast.xml instead of LATEST

Post by eddyg »

The handbrake version number was removed when using an SVN build, as it doesn't really mean anything.
eddyg
Veteran User
Posts: 798
Joined: Mon Apr 23, 2007 3:34 am

Re: update.c using appcast.xml instead of LATEST

Post by eddyg »

Code: Select all

Index: update.c
===================================================================
--- update.c   (revision 1402)
+++ update.c   (working copy)
@@ -7,7 +7,7 @@
#include "hb.h"

#define HB_URL   "handbrake.fr"
-#define HB_QUERY "GET /LATEST HTTP/1.0\r\nHost: " HB_URL "\r\n\r\n"
+#define HB_QUERY "GET /appcast.xml HTTP/1.0\r\nHost: " HB_URL "\r\n\r\n"

typedef struct
{
@@ -92,45 +92,96 @@
     {
         goto error;
     }
+   
+   
+   
+   
+   
+   
+   /*
+    * Find the <cli-stable> tag
+    * Scan though each character of the buffer until we find that the first 4 characters of "cur" are "<cli"
+    */

-    stable = strtol( cur, &p, 10 );
-    if( cur == p )
-    {
-        goto error;
-    }
-    cur = p + 1;
-    memset( stable_str, 0, sizeof( stable_str ) );
-    for( i = 0;
-         i < sizeof( stable_str ) - 1 && cur < end && *cur != '\n';
-         i++, cur++ )
-    {
-        stable_str[i] = *cur;
-    }
-
+     for( ; &cur[3] < end; cur++ )
+     {
+        if( cur[0] == '<' && cur[1] == 'c' && cur[2] == 'l' && cur[3] == 'i' )
+         {
+            cur += 1;
+            break;
+         }
+     }
+   
+   /*
+    * Ok, The above code didn't position cur, it only found <cli so we need to shift cur along 11 places.
+    * After which, the next 10 characters are the build number
+    */
+    cur += 11;
You should check that cur + 11 is not greater than end here to avoid going off into the woods.

Code: Select all

+   stable = strtol( cur, &cur, 10 );
+   printf("Stable Build: %i \n", stable);

Are we sure that the buffer is null terminated? Otherwise strtol could walk off of the end of it.

Note that at this point cur is at an undefined point, possibly at the end of the string if no number was found.

Code: Select all

+   
+   /*
+    * The Version number is 2 places after the build, so shift cur, 2 places.
+    * Get all the characters in cur until the point where " is found.
+    */
+   cur += 2;
Again we need to check that we haven't walked off of the "end" of the buffer.

Code: Select all

+   for( i = 0;   i < sizeof( stable_str ) - 1 && cur < end && *cur != '"'; i++, cur++ )
+   {
+      stable_str[i] = *cur;
+   }
+   printf("Stable Version: %s \n", stable_str);
What is *cur != "" supposed to mean? Should that be *cur != '\0' for detecting the null terminator?

<edit> the above was made clearer when a different font was being used, it was actually '"'. so not a problem - sorry.

Same comments as above apply to the code below.

Also - when searching you should be able to handle not finding what you are looking for gracefully.

Code: Select all

+   
     hb_log( "latest stable: %s, build %d", stable_str, stable );
+   

-    cur++;
-    if( cur >= end )
-    {
-        goto error;
-    }

-    unstable = strtol( cur, &p, 10 );
-    if( cur == p )
-    {
-        goto error;
-    }
-    cur = p + 1;
-    memset( unstable_str, 0, sizeof( unstable_str ) );
-    for( i = 0;
-         i < sizeof( unstable_str ) - 1 && cur < end && *cur != '\n';
-         i++, cur++ )
-    {
-        unstable_str[i] = *cur;
-    }

+
+
+   /*
+    * Find the <cli-unstable> tag
+    * Scan though each character of the buffer until we find that the first 4 characters of "cur" are "<cli"
+    */
+
+     for( ; &cur[3] < end; cur++ )
+     {
+        if( cur[0] == '<' && cur[1] == 'c' && cur[2] == 'l' && cur[3] == 'i' )
+         {
+            cur += 1;
+            break;
+         }
+     }
If we didn't find "<cli" we may be about to crash when we jump 13 chrs past the end of buffer and do a strtol.

Code: Select all

+   
+   /*
+    * Now we need to handle the unstable build information
+    * Unstable build number is 29 Characters after the last position used.
+    */
+    
+    cur += 13;     
+    unstable = strtol( cur, &p, 10 );
+    printf("Unstable Build: %i \n", unstable);
+   
+   /*
+    * Now we need to get the unstable version number.
+    * First move the cur pointer 12 places.
+    * Then iterate over cur until " is found. Thats the end of the version number.
+    */
+   cur += 12;
+   for( i = 0;   i < sizeof( unstable_str ) - 1 && cur < end && *cur != '"'; i++, cur++ )
+   {
+      unstable_str[i] = *cur;
+   }
+   printf("Unstable Version: %s \n", unstable_str);
+   printf("\n\n");
+   
+
     hb_log( "latest unstable: %s, build %d", unstable_str, unstable );

+
+   /*
+    * Handle the update checking as normal.
+    * This code is unchanged.
+    */
     if( HB_BUILD % 100 )
     {
         /* We are runnning an unstable build */
saintdev
Enlightened
Posts: 146
Joined: Wed Dec 20, 2006 4:17 am

Re: update.c using appcast.xml instead of LATEST

Post by saintdev »

sr55 wrote:

Code: Select all

Index: update.c
===================================================================
--- update.c	(revision 1402)
+++ update.c	(working copy)
@@ -7,7 +7,7 @@
 #include "hb.h"
 
 #define HB_URL   "handbrake.fr"
-#define HB_QUERY "GET /LATEST HTTP/1.0\r\nHost: " HB_URL "\r\n\r\n"
+#define HB_QUERY "GET /appcast.xml HTTP/1.0\r\nHost: " HB_URL "\r\n\r\n"
 
 typedef struct
 {
@@ -92,45 +92,96 @@
     {
         goto error;
     }
+	
+	
+	
+	
+	
+	
+	/*
+	 * Find the <cli-stable> tag
+	 * Scan though each character of the buffer until we find that the first 4 characters of "cur" are "<cli"
+	 */
 
-    stable = strtol( cur, &p, 10 );
-    if( cur == p )
-    {
-        goto error;
-    }
-    cur = p + 1;
-    memset( stable_str, 0, sizeof( stable_str ) );
-    for( i = 0;
-         i < sizeof( stable_str ) - 1 && cur < end && *cur != '\n';
-         i++, cur++ )
-    {
-        stable_str[i] = *cur;
-    }
-
+     for( ; &cur[3] < end; cur++ )
+     {
+        if( cur[0] == '<' && cur[1] == 'c' && cur[2] == 'l' && cur[3] == 'i' )
+         {
+            cur += 1;
+            break;
+         }
+     }
strstr? Or even better strcasestr..

Code: Select all

+	/*
+	 * Ok, The above code didn't position cur, it only found <cli so we need to shift cur along 11 places.
+	 * After which, the next 10 characters are the build number
+	 */
+    cur += 11;
+	stable = strtol( cur, &cur, 10 );
+	printf("Stable Build: %i \n", stable);
+	
+	/*
+	 * The Version number is 2 places after the build, so shift cur, 2 places.
+	 * Get all the characters in cur until the point where " is found.
+	 */
+	cur += 2;
+	for( i = 0;   i < sizeof( stable_str ) - 1 && cur < end && *cur != '"'; i++, cur++ )
+	{
+		stable_str[i] = *cur;
+	}
+	printf("Stable Version: %s \n", stable_str);
+	
     hb_log( "latest stable: %s, build %d", stable_str, stable );
+	
 
-    cur++;
-    if( cur >= end )
-    {
-        goto error;
-    }
 
-    unstable = strtol( cur, &p, 10 );
-    if( cur == p )
-    {
-        goto error;
-    }
-    cur = p + 1;
-    memset( unstable_str, 0, sizeof( unstable_str ) );
-    for( i = 0;
-         i < sizeof( unstable_str ) - 1 && cur < end && *cur != '\n';
-         i++, cur++ )
-    {
-        unstable_str[i] = *cur;
-    }
 
+
+
+	/*
+	 * Find the <cli-unstable> tag
+	 * Scan though each character of the buffer until we find that the first 4 characters of "cur" are "<cli"
+	 */
+
+     for( ; &cur[3] < end; cur++ )
+     {
+        if( cur[0] == '<' && cur[1] == 'c' && cur[2] == 'l' && cur[3] == 'i' )
+         {
+            cur += 1;
+            break;
+         }
+     }
+	 
+	/*
+	 * Now we need to handle the unstable build information
+	 * Unstable build number is 29 Characters after the last position used.
+	 */
+	 
+	 cur += 13;     
+	 unstable = strtol( cur, &p, 10 );
+	 printf("Unstable Build: %i \n", unstable);
+	
+	/*
+	 * Now we need to get the unstable version number.
+	 * First move the cur pointer 12 places.
+	 * Then iterate over cur until " is found. Thats the end of the version number.
+	 */
+	cur += 12;
+	for( i = 0;   i < sizeof( unstable_str ) - 1 && cur < end && *cur != '"'; i++, cur++ )
+	{
+		unstable_str[i] = *cur;
+	}
+	printf("Unstable Version: %s \n", unstable_str);
+	printf("\n\n");
+	
+
     hb_log( "latest unstable: %s, build %d", unstable_str, unstable );
 
+
+	/*
+	 * Handle the update checking as normal.
+	 * This code is unchanged.
+	 */
     if( HB_BUILD % 100 )
     {
         /* We are runnning an unstable build */
Probably good places to use strtok (or strtok_r). Using hard-coded positions isn't really the greatest idea.

Code: Select all

<cli-stable>2009021900 "0.9.4" </cli-stable>
	  <cli-unstable>2009021901 "0.9.3" </cli-unstable>
What about something like:

Code: Select all

<cli buildtype="stable">
    <build>2009021900</build>
    <version>0.9.4</version>
</cli>
<cli buildtype="unstable">
    <build>2009021901</build>
    <version>0.9.3</version>
</cli>
A little more proper-like XML, IMO.

Code: Select all

memset( unstable_str, 0, sizeof( unstable_str ) );
that I removed. Not really sure what they do, and even if they are in fact needed any more?
They make sure the memory contained therin is initalized to something, other than being random leftovers from whatever was there last.
User avatar
s55
HandBrake Team
Posts: 10350
Joined: Sun Dec 24, 2006 1:05 pm

Re: update.c using appcast.xml instead of LATEST

Post by s55 »

Code: Select all

Index: update.c[quote]
===================================================================
--- update.c	(revision 1402)
+++ update.c	(working copy)
@@ -7,7 +7,7 @@
 #include "hb.h"
 
 #define HB_URL   "handbrake.fr"
-#define HB_QUERY "GET /LATEST HTTP/1.0\r\nHost: " HB_URL "\r\n\r\n"
+#define HB_QUERY "GET /appcast.xml HTTP/1.0\r\nHost: " HB_URL "\r\n\r\n"
 
 typedef struct
 {
@@ -92,45 +92,157 @@
     {
         goto error;
     }
+	[/quote]
+	
+	// FIND THE STABLE VERSION INFORMATION ###################################################
+	
+	/*
+	 * Find the <cli-stable> tag
+	 * Scan though each character of the buffer until we find that the first 4 characters of "cur" are "<cli"
+	 */
 
-    stable = strtol( cur, &p, 10 );
-    if( cur == p )
+     for(i=0 ; &cur[3] < end; i++, cur++ )
+     {
+        if( cur[0] == '<' && cur[1] == 'c' && cur[2] == 'l' && cur[3] == 'i' )
+         {
+            cur += 1;
+            break;
+         }
+		 
+		 // If the CLI tag has not been found in the first 510 characters, or the end is reached, something bad happened.
+		 if (( i > 510) || ( cur >= end ))
+		 {
+		 	goto error;
+		 }
+     }
+	 
+	if( cur >= end )
     {
         goto error;
     }
-    cur = p + 1;
-    memset( stable_str, 0, sizeof( stable_str ) );
-    for( i = 0;
-         i < sizeof( stable_str ) - 1 && cur < end && *cur != '\n';
-         i++, cur++ )
+	
+	/*
+	 * Ok, The above code didn't position cur, it only found <cli so we need to shift cur along 11 places.
+	 * After which, the next 10 characters are the build number
+	 */
+    cur += 11;
+	
+	if( cur >= end )
     {
-        stable_str[i] = *cur;
+        goto error;
     }
-
-    hb_log( "latest stable: %s, build %d", stable_str, stable );
-
-    cur++;
-    if( cur >= end )
+	
+	stable = strtol( cur, &cur, 10 );
+	
+	if( cur >= end )
     {
         goto error;
     }
+	
+	/*
+	 * The Version number is 2 places after the build, so shift cur, 2 places.
+	 * Get all the characters in cur until the point where " is found.
+	 */
+	cur += 2;
+	
+	if( cur >= end )
+    {
+        goto error;
+    }
+	memset( stable_str, 0, sizeof( stable_str ) );
+	for( i = 0;   i < sizeof( stable_str ) - 1 && cur < end && *cur != '"'; i++, cur++ )
+	{
+		stable_str[i] = *cur;
+		
+		// If the version number is longer than 7 characters, or the end is reached, something has gone wrong.
+		if (( i > 7) || ( cur >= end ))
+		{
+			goto error;
+		}
+	}
+	
+	if( cur >= end )
+    {
+        goto error;
+    }
+	
+    hb_log( "latest stable: %s, build %d", stable_str, stable );
+	
+	// END OF STABLE INFO ###################################################
+	
 
-    unstable = strtol( cur, &p, 10 );
-    if( cur == p )
+	// FIND THE UNSTABLE INFO ###############################################
+	/*
+	 * Find the <cli-unstable> tag
+	 * Scan though each character of the buffer until we find that the first 4 characters of "cur" are "<cli"
+	 */
+
+     for(i =0 ; &cur[3] < end; i++, cur++ )
+     {
+        if( cur[0] == '<' && cur[1] == 'c' && cur[2] == 'l' && cur[3] == 'i' )
+         {
+            cur += 1;
+            break;
+         }
+		 
+		 // If the second CLI tag is more than 25 characters forward, or the end is reached, something went wrong.
+		 if (( i > 25) || ( cur >= end ))
+		 {
+		 	goto error;
+		 }
+     }
+	 
+	/*
+	 * Now we need to handle the unstable build information
+	 * Unstable build number is 29 Characters after the last position used.
+	 */
+	 
+	 cur += 13;
+	     
+	if( cur >= end )
     {
         goto error;
+    } 
+	
+	 unstable = strtol( cur, &p, 10 );
+	 
+	if( cur >= end )
+    {
+        goto error;
     }
-    cur = p + 1;
-    memset( unstable_str, 0, sizeof( unstable_str ) );
-    for( i = 0;
-         i < sizeof( unstable_str ) - 1 && cur < end && *cur != '\n';
-         i++, cur++ )
+	
+	/*
+	 * Now we need to get the unstable version number.
+	 * First move the cur pointer 12 places.
+	 * Then iterate over cur until " is found. Thats the end of the version number.
+	 */
+	cur += 12;
+	
+	if( cur >= end )
     {
-        unstable_str[i] = *cur;
+        goto error;
     }
+	
+	memset( unstable_str, 0, sizeof( unstable_str ) );
+	for( i = 0;   i < sizeof( unstable_str ) - 1 && cur < end && *cur != '"'; i++, cur++ )
+	{
+		unstable_str[i] = *cur;
+		
+		// If the version number is greater than 7 chars or the end is reached, something went wrong.
+		if (( i > 7) || ( cur >= end ))
+		{
+			goto error;
+		}
+	}
 
     hb_log( "latest unstable: %s, build %d", unstable_str, unstable );
+	
+	// END OF UNSTABLE INFO ###################################################
 
+	/*
+	 * Handle the update checking as normal.
+	 * This code is unchanged.
+	 */
     if( HB_BUILD % 100 )
     {
         /* We are runnning an unstable build */
- Added the if ( cur >= end ) { goto error; } lines back in.
- Also added them to the for loops + a check to make sure we didn't try to take in too many characters for a version number, which could happen if " was missing.
- Added the memset lines back in since saintdev made them sound important.
Probably good places to use strtok (or strtok_r). Using hard-coded positions isn't really the greatest idea.
True, but that's what was used previously, The file format is not going to change now so it's effort which won't make a great deal of difference. Once the first CLI tag is found, nothing will change that the searching code doesn't already handle.

A little more proper-like XML, IMO.
The reason I didn't do that, is because it's a pain to sort out my GUI to handle it. It's easier just to have the version number as a single string.
They make sure the memory contained therin is initalized to something, other than being random leftovers from whatever was there last.
Sounds important. I've re-added those lines in.

Now for eddyg's comments.
You should check that cur + 11 is not greater than end here to avoid going off into the woods.
Done. ( I think) ALong with the rest of the checks have been re-added.
Are we sure that the buffer is null terminated? Otherwise strtol could walk off of the end of it.
*shrugs*. The old update.c used strtol, so that's why I used it. I saw no reason to change what worked. I suppose I could do something like
http://www.daniweb.com/forums/post254197-3.html
but tbh, I can't really be bothered when this seems to work fine.
Also - when searching you should be able to handle not finding what you are looking for gracefully.
Yes, I've sorted this now. I've set a limit on the number of loops that can happen, and also make sure that the code hasn't reached the end of cur (if some really really weird something happens)
eddyg
Veteran User
Posts: 798
Joined: Mon Apr 23, 2007 3:34 am

Re: update.c using appcast.xml instead of LATEST

Post by eddyg »

That's a lot better.

The only worry (for me) is using strtol() when you can't guarantee that the buffer is null terminated.

Maybe you should make sure that buffer[end] = '\0' if that is not already ensured (sorry I was too lazy to check myself)?

Cheers, Ed.
User avatar
s55
HandBrake Team
Posts: 10350
Joined: Sun Dec 24, 2006 1:05 pm

Re: update.c using appcast.xml instead of LATEST

Post by s55 »

printf("BUFFER END %s ", cur[1024]);

returns

Code: Select all

BUFFER END (null)
I assume that is what we wanted to see?

Uploaded a patch taken from the trunk directory.
Attachments

[The extension patch has been deactivated and can no longer be displayed.]

Post Reply