[MPlayer-dev-eng] ctype.h functions on win32

Gianluigi Tiesi mplayer at netfarm.it
Sun Oct 14 17:25:42 CEST 2007


On Sun, Oct 14, 2007 at 01:01:18AM -0400, Rich Felker wrote:
> > -	char *tmp = realloc(*lineptr, 4096);
> > +	unsigned char *tmp = (unsigned char *) realloc(*lineptr, 4096);
> 
> Never cast return values of malloc/realloc.
> 

uh? I alwaus used to cast them, I was wrong? :)
anyway removed as your request

> >  {
> >      // org: X,Y
> >      char *p;
> 
> Is this p used incorrectly?

yes strtoul accepts char *

> > Index: stream/tv.c
> > ===================================================================
> > --- stream/tv.c	(revision 24775)
> > +++ stream/tv.c	(working copy)
> > @@ -484,7 +484,7 @@
> >  	int channel = 0;
> >  	if (tvh->tv_param->channel)
> >  	 {
> > -	   if (isdigit(*tvh->tv_param->channel))
> > +	   if (isdigit((uint8_t) *tvh->tv_param->channel))
> >  		/* if tvh->tv_param->channel begins with a digit interpret it as a number */
> >  		channel = atoi(tvh->tv_param->channel);
> 
> unsigned char is a better name for it.

done


> > Index: input/input.c
> > ===================================================================
> > --- input/input.c	(revision 24775)
> > +++ input/input.c	(working copy)
> > @@ -1353,7 +1353,7 @@
> >        return key_names[i].name;
> >    }
> >    
> > -  if(isascii(key)) {
> > +  if(isascii((unsigned char) key)) {
> 
> isascii is not terribly portable to begin with. Just coding the test
> explicitly would make more sense...


leaved input.c as is, it's a bit unclear for me

> > Index: dvdread/ifo_print.c
> > ===================================================================
> > --- dvdread/ifo_print.c	(revision 24775)
> > +++ dvdread/ifo_print.c	(working copy)
> > @@ -403,8 +403,8 @@
> >    }    
> >   
> >    if(attr->type == 1) {
> > -    if(isalpha((int)(attr->lang_code >> 8))
> > -       && isalpha((int)(attr->lang_code & 0xff))) {
> > +    if(isalpha((unsigned int)(attr->lang_code >> 8))
> > +       && ((unsigned int)(attr->lang_code & 0xff))) {
> 
> This change looks nonsensical.
> 

You are right, removed

> > Index: libvo/gl_common.c
> > ===================================================================
> > --- libvo/gl_common.c	(revision 24775)
> > +++ libvo/gl_common.c	(working copy)
> > @@ -389,7 +389,7 @@
> >   */
> >  int glCreatePPMTex(GLenum target, GLenum fmt, GLint filter,
> >                     FILE *f, int *width, int *height, int *maxval) {
> > -  unsigned w, h, m, val, bpp;
> > +  unsigned w, h, m, bpp;
> >    char *data;
> >    ppm_skip(f);
> >    if (fgetc(f) != 'P' || fgetc(f) != '6')
> > @@ -403,8 +403,7 @@
> >    ppm_skip(f);
> >    if (fscanf(f, "%u", &m) != 1)
> >      return 0;
> > -  val = fgetc(f);
> > -  if (!isspace(val))
> > +  if (!isspace(fgetc(f)))
> 
> I think this is okay... is isspace guaranteed not to evaluate its
> argument more than once?

removed, it's supposed that fgetc result is ok

> 
> > -  if(isascii(c)) {
> > +  if(isascii((uint8_t) c)) {
> 
> if (!(c&0x80))
> 

unclear for me, removed the patch

> > Index: drivers/mga_vid.c
> > ===================================================================
> > --- drivers/mga_vid.c	(revision 24775)
> > +++ drivers/mga_vid.c	(working copy)
> > @@ -121,7 +121,7 @@
> >  #if LINUX_VERSION_CODE < KERNEL_VERSION(2,4,0)
> >  #include <linux/ctype.h>
> >  
> > -static unsigned long simple_strtoul(const char *cp,char **endp,unsigned int base)
> > +static unsigned long simple_strtoul(const unsigned char *cp,char **endp,unsigned int base)
> >  {
> >          unsigned long result = 0,value;
> 
> Seems unrelated, this is Linux-kernel-specific code using
> linux/ctype.h not ctype.h.

sorry, my fault, removed

> > -double ff_eval2(char *s, double *const_value, const char **const_name,
> > +double ff_eval2(unsigned char *s, double *const_value, const char **const_name,
> 
> Changing api here is unfriendly to the caller. Better would be to cast
> the pointer in the implementation of this function if needed.
> 
> > -attribute_deprecated double ff_eval(char *s, double *const_value, const char **const_name,
> > +attribute_deprecated double ff_eval(unsigned char *s, double *const_value, const char **const_name,
> 
> Ditto.
> 
> > -int av_get_frame_filename(char *buf, int buf_size,
> > +int av_get_frame_filename(unsigned char *buf, int buf_size,

you are right, I've removed the patch, added a cast

I'm attaching the revised patches

-- 
Gianluigi Tiesi <sherpya at netfarm.it>
EDP Project Leader
Netfarm S.r.l. - http://www.netfarm.it/
Free Software: http://oss.netfarm.it/



More information about the MPlayer-dev-eng mailing list