Like free(), ksh's afree() is NULL-safe

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Like free(), ksh's afree() is NULL-safe

Michael McConville-2
Also, why is fs on line 390 cast to char* when afree() takes void*?


Index: emacs.c
===================================================================
RCS file: /cvs/src/bin/ksh/emacs.c,v
retrieving revision 1.50
diff -u -p -r1.50 emacs.c
--- emacs.c 25 Mar 2015 12:10:52 -0000 1.50
+++ emacs.c 31 Aug 2015 04:02:29 -0000
@@ -1307,8 +1307,7 @@ static void
 kb_del(struct kb_entry *k)
 {
  TAILQ_REMOVE(&kblist, k, entry);
- if (k->args)
- free(k->args);
+ free(k->args);
  afree(k, AEDIT);
 }
 
Index: var.c
===================================================================
RCS file: /cvs/src/bin/ksh/var.c,v
retrieving revision 1.42
diff -u -p -r1.42 var.c
--- var.c 19 Aug 2015 16:05:46 -0000 1.42
+++ var.c 31 Aug 2015 04:02:29 -0000
@@ -385,8 +385,7 @@ setstr(struct tbl *vq, const char *s, in
  vq->flag |= ISSET;
  if ((vq->flag&SPECIAL))
  setspec(vq);
- if (fs)
- afree((char *)fs, ATEMP);
+ afree((char *)fs, ATEMP);
  return 1;
 }
 
@@ -576,8 +575,7 @@ export(struct tbl *vp, const char *val)
  *xp++ = '=';
  vp->type = xp - vp->val.s; /* offset to value */
  memcpy(xp, val, vallen);
- if (op != NULL)
- afree((void*)op, vp->areap);
+ afree((void*)op, vp->areap);
 }
 
 /*
@@ -704,8 +702,7 @@ typeset(const char *var, Tflag set, Tfla
  t->type = 0;
  }
  }
- if (free_me)
- afree((void *) free_me, t->areap);
+ afree((void *) free_me, t->areap);
  }
  }
  if (!ok)
@@ -952,8 +949,7 @@ setspec(struct tbl *vp)
 
  switch (special(vp->name)) {
  case V_PATH:
- if (path)
- afree(path, APERM);
+ afree(path, APERM);
  path = str_save(str_val(vp), APERM);
  flushcom(1); /* clear tracked aliases */
  break;
@@ -1059,8 +1055,7 @@ unsetspec(struct tbl *vp)
 {
  switch (special(vp->name)) {
  case V_PATH:
- if (path)
- afree(path, APERM);
+ afree(path, APERM);
  path = str_save(def_path, APERM);
  flushcom(1); /* clear tracked aliases */
  break;

Reply | Threaded
Open this post in threaded view
|

Re: Like free(), ksh's afree() is NULL-safe

Ted Unangst-6
Michael McConville wrote:
> Also, why is fs on line 390 cast to char* when afree() takes void*?

this code is older than void. and NULL apparently. please remove the casts
too.

Reply | Threaded
Open this post in threaded view
|

Re: Like free(), ksh's afree() is NULL-safe

Michael McConville-2
Ted Unangst wrote:
> Michael McConville wrote:
> > Also, why is fs on line 390 cast to char* when afree() takes void*?
>
> this code is older than void. and NULL apparently. please remove the casts
> too.

Does this look good? I had to leave one cast because the freed pointer
is a const.


Index: c_ksh.c
===================================================================
RCS file: /cvs/src/bin/ksh/c_ksh.c,v
retrieving revision 1.34
diff -u -p -r1.34 c_ksh.c
--- c_ksh.c 17 Dec 2013 16:37:05 -0000 1.34
+++ c_ksh.c 1 Sep 2015 00:22:03 -0000
@@ -943,7 +943,7 @@ c_alias(char **wp)
  if ((val && !tflag) || (!val && tflag && !Uflag)) {
  if (ap->flag&ALLOC) {
  ap->flag &= ~(ALLOC|ISSET);
- afree((void*)ap->val.s, APERM);
+ afree(ap->val.s, APERM);
  }
  /* ignore values for -t (at&t ksh does this) */
  newval = tflag ? search(alias, path, X_OK, (int *) 0) :
@@ -998,7 +998,7 @@ c_unalias(char **wp)
  }
  if (ap->flag&ALLOC) {
  ap->flag &= ~(ALLOC|ISSET);
- afree((void*)ap->val.s, APERM);
+ afree(ap->val.s, APERM);
  }
  ap->flag &= ~(DEFINED|ISSET|EXPORT);
  }
@@ -1009,7 +1009,7 @@ c_unalias(char **wp)
  for (ktwalk(&ts, t); (ap = ktnext(&ts)); ) {
  if (ap->flag&ALLOC) {
  ap->flag &= ~(ALLOC|ISSET);
- afree((void*)ap->val.s, APERM);
+ afree(ap->val.s, APERM);
  }
  ap->flag &= ~(DEFINED|ISSET|EXPORT);
  }
Index: edit.c
===================================================================
RCS file: /cvs/src/bin/ksh/edit.c,v
retrieving revision 1.40
diff -u -p -r1.40 edit.c
--- edit.c 12 Mar 2015 10:20:30 -0000 1.40
+++ edit.c 1 Sep 2015 00:22:04 -0000
@@ -489,7 +489,7 @@ x_command_glob(int flags, const char *st
  path_order_cmp);
  for (i = 0; i < nwords; i++)
  words[i] = info[i].word;
- afree((void *) info, ATEMP);
+ afree(info, ATEMP);
  } else {
  /* Sort and remove duplicate entries */
  char **words = (char **) XPptrv(w);
Index: emacs.c
===================================================================
RCS file: /cvs/src/bin/ksh/emacs.c,v
retrieving revision 1.50
diff -u -p -r1.50 emacs.c
--- emacs.c 25 Mar 2015 12:10:52 -0000 1.50
+++ emacs.c 1 Sep 2015 00:22:04 -0000
@@ -1149,7 +1149,7 @@ x_push(int nchars)
 {
  char *cp = str_nsave(xcp, nchars, AEDIT);
  if (killstack[killsp])
- afree((void *)killstack[killsp], AEDIT);
+ afree(killstack[killsp], AEDIT);
  killstack[killsp] = cp;
  killsp = (killsp + 1) % KILLSIZE;
 }
@@ -1307,8 +1307,7 @@ static void
 kb_del(struct kb_entry *k)
 {
  TAILQ_REMOVE(&kblist, k, entry);
- if (k->args)
- free(k->args);
+ free(k->args);
  afree(k, AEDIT);
 }
 
Index: expand.h
===================================================================
RCS file: /cvs/src/bin/ksh/expand.h,v
retrieving revision 1.6
diff -u -p -r1.6 expand.h
--- expand.h 30 Mar 2005 17:16:37 -0000 1.6
+++ expand.h 1 Sep 2015 00:22:04 -0000
@@ -55,7 +55,7 @@ typedef char * XStringP;
 #define Xcheck(xs, xp) XcheckN(xs, xp, 1)
 
 /* free string */
-#define Xfree(xs, xp) afree((void*) (xs).beg, (xs).areap)
+#define Xfree(xs, xp) afree((xs).beg, (xs).areap)
 
 /* close, return string */
 #define Xclose(xs, xp) (char*) aresize((void*)(xs).beg, \
@@ -104,4 +104,4 @@ typedef struct XPtrV {
 #define XPclose(x) (void**) aresize((void*)(x).beg, \
  sizeofN(void*, XPsize(x)), ATEMP)
 
-#define XPfree(x) afree((void*) (x).beg, ATEMP)
+#define XPfree(x) afree((x).beg, ATEMP)
Index: history.c
===================================================================
RCS file: /cvs/src/bin/ksh/history.c,v
retrieving revision 1.40
diff -u -p -r1.40 history.c
--- history.c 20 Nov 2014 15:22:39 -0000 1.40
+++ history.c 1 Sep 2015 00:22:04 -0000
@@ -417,7 +417,7 @@ histbackup(void)
 
  if (histptr >= history && last_line != hist_source->line) {
  hist_source->line--;
- afree((void*)*histptr, APERM);
+ afree(*histptr, APERM);
  histptr--;
  last_line = hist_source->line;
  }
@@ -589,7 +589,7 @@ histsave(int lno, const char *cmd, int d
  hp = histptr;
 
  if (++hp >= history + histsize) { /* remove oldest command */
- afree((void*)*history, APERM);
+ afree(*history, APERM);
  for (hp = history; hp < history + histsize - 1; hp++)
  hp[0] = hp[1];
  }
@@ -872,7 +872,7 @@ histinsert(Source *s, int lno, unsigned
  if (lno >= s->line-(histptr-history) && lno <= s->line) {
  hp = &histptr[lno-s->line];
  if (*hp)
- afree((void*)*hp, APERM);
+ afree(*hp, APERM);
  *hp = str_save((char *)line, APERM);
  }
 }
Index: mail.c
===================================================================
RCS file: /cvs/src/bin/ksh/mail.c,v
retrieving revision 1.17
diff -u -p -r1.17 mail.c
--- mail.c 28 Nov 2013 10:33:37 -0000 1.17
+++ mail.c 1 Sep 2015 00:22:04 -0000
@@ -90,9 +90,9 @@ mbset(char *p)
  struct stat stbuf;
 
  if (mbox.mb_msg)
- afree((void *)mbox.mb_msg, APERM);
+ afree(mbox.mb_msg, APERM);
  if (mbox.mb_path)
- afree((void *)mbox.mb_path, APERM);
+ afree(mbox.mb_path, APERM);
  /* Save a copy to protect from export (which munges the string) */
  mbox.mb_path = str_save(p, APERM);
  mbox.mb_msg = NULL;
@@ -151,8 +151,8 @@ munset(mbox_t *mlist)
  mbp = mlist;
  mlist = mbp->mb_next;
  if (!mlist)
- afree((void *)mbp->mb_path, APERM);
- afree((void *)mbp, APERM);
+ afree(mbp->mb_path, APERM);
+ afree(mbp, APERM);
  }
 }
 
Index: syn.c
===================================================================
RCS file: /cvs/src/bin/ksh/syn.c,v
retrieving revision 1.29
diff -u -p -r1.29 syn.c
--- syn.c 3 Jun 2013 18:40:05 -0000 1.29
+++ syn.c 1 Sep 2015 00:22:04 -0000
@@ -206,7 +206,7 @@ get_command(int cf)
  switch (c = token(cf|KEYWORD|ALIAS|VARASN)) {
  default:
  REJECT;
- afree((void*) iops, ATEMP);
+ afree(iops, ATEMP);
  XPfree(args);
  XPfree(vars);
  return NULL; /* empty line */
@@ -385,7 +385,7 @@ get_command(int cf)
  }
 
  if (iopn == 0) {
- afree((void*) iops, ATEMP);
+ afree(iops, ATEMP);
  t->ioact = NULL;
  } else {
  iops[iopn++] = NULL;
Index: table.c
===================================================================
RCS file: /cvs/src/bin/ksh/table.c,v
retrieving revision 1.15
diff -u -p -r1.15 table.c
--- table.c 19 Feb 2012 07:52:30 -0000 1.15
+++ table.c 1 Sep 2015 00:22:04 -0000
@@ -58,10 +58,10 @@ texpand(struct table *tp, int nsize)
  *p = tblp;
  tp->nfree--;
  } else if (!(tblp->flag & FINUSE)) {
- afree((void*)tblp, tp->areap);
+ afree(tblp, tp->areap);
  }
  }
- afree((void*)otblp, tp->areap);
+ afree(otblp, tp->areap);
 }
 
 /* table */
Index: tree.c
===================================================================
RCS file: /cvs/src/bin/ksh/tree.c,v
retrieving revision 1.20
diff -u -p -r1.20 tree.c
--- tree.c 27 Jun 2012 07:17:19 -0000 1.20
+++ tree.c 1 Sep 2015 00:22:04 -0000
@@ -666,18 +666,18 @@ tfree(struct op *t, Area *ap)
  return;
 
  if (t->str != NULL)
- afree((void*)t->str, ap);
+ afree(t->str, ap);
 
  if (t->vars != NULL) {
  for (w = t->vars; *w != NULL; w++)
- afree((void*)*w, ap);
- afree((void*)t->vars, ap);
+ afree(*w, ap);
+ afree(t->vars, ap);
  }
 
  if (t->args != NULL) {
  for (w = t->args; *w != NULL; w++)
- afree((void*)*w, ap);
- afree((void*)t->args, ap);
+ afree(*w, ap);
+ afree(t->args, ap);
  }
 
  if (t->ioact != NULL)
@@ -686,7 +686,7 @@ tfree(struct op *t, Area *ap)
  tfree(t->left, ap);
  tfree(t->right, ap);
 
- afree((void*)t, ap);
+ afree(t, ap);
 }
 
 static void
@@ -697,12 +697,12 @@ iofree(struct ioword **iow, Area *ap)
 
  for (iop = iow; (p = *iop++) != NULL; ) {
  if (p->name != NULL)
- afree((void*)p->name, ap);
+ afree(p->name, ap);
  if (p->delim != NULL)
- afree((void*)p->delim, ap);
+ afree(p->delim, ap);
  if (p->heredoc != NULL)
- afree((void*)p->heredoc, ap);
- afree((void*)p, ap);
+ afree(p->heredoc, ap);
+ afree(p, ap);
  }
  afree(iow, ap);
 }
Index: var.c
===================================================================
RCS file: /cvs/src/bin/ksh/var.c,v
retrieving revision 1.42
diff -u -p -r1.42 var.c
--- var.c 19 Aug 2015 16:05:46 -0000 1.42
+++ var.c 1 Sep 2015 00:22:04 -0000
@@ -366,7 +366,7 @@ setstr(struct tbl *vq, const char *s, in
  internal_errorf(true,
     "setstr: %s=%s: assigning to self",
     vq->name, s);
- afree((void*)vq->val.s, vq->areap);
+ afree(vq->val.s, vq->areap);
  }
  vq->flag &= ~(ISSET|ALLOC);
  vq->type = 0;
@@ -385,8 +385,7 @@ setstr(struct tbl *vq, const char *s, in
  vq->flag |= ISSET;
  if ((vq->flag&SPECIAL))
  setspec(vq);
- if (fs)
- afree((char *)fs, ATEMP);
+ afree((void *)fs, ATEMP);
  return 1;
 }
 
@@ -576,8 +575,7 @@ export(struct tbl *vp, const char *val)
  *xp++ = '=';
  vp->type = xp - vp->val.s; /* offset to value */
  memcpy(xp, val, vallen);
- if (op != NULL)
- afree((void*)op, vp->areap);
+ afree(op, vp->areap);
 }
 
 /*
@@ -698,14 +696,12 @@ typeset(const char *var, Tflag set, Tfla
  t->flag &= ~ISSET;
  else {
  if (t->flag & ALLOC)
- afree((void*) t->val.s,
-    t->areap);
+ afree(t->val.s, t->areap);
  t->flag &= ~(ISSET|ALLOC);
  t->type = 0;
  }
  }
- if (free_me)
- afree((void *) free_me, t->areap);
+ afree(free_me, t->areap);
  }
  }
  if (!ok)
@@ -739,7 +735,7 @@ void
 unset(struct tbl *vp, int array_ref)
 {
  if (vp->flag & ALLOC)
- afree((void*)vp->val.s, vp->areap);
+ afree(vp->val.s, vp->areap);
  if ((vp->flag & ARRAY) && !array_ref) {
  struct tbl *a, *tmp;
 
@@ -748,7 +744,7 @@ unset(struct tbl *vp, int array_ref)
  tmp = a;
  a = a->u.array;
  if (tmp->flag & ALLOC)
- afree((void *) tmp->val.s, tmp->areap);
+ afree(tmp->val.s, tmp->areap);
  afree(tmp, tmp->areap);
  }
  vp->u.array = (struct tbl *) 0;
@@ -952,8 +948,7 @@ setspec(struct tbl *vp)
 
  switch (special(vp->name)) {
  case V_PATH:
- if (path)
- afree(path, APERM);
+ afree(path, APERM);
  path = str_save(str_val(vp), APERM);
  flushcom(1); /* clear tracked aliases */
  break;
@@ -1059,8 +1054,7 @@ unsetspec(struct tbl *vp)
 {
  switch (special(vp->name)) {
  case V_PATH:
- if (path)
- afree(path, APERM);
+ afree(path, APERM);
  path = str_save(def_path, APERM);
  flushcom(1); /* clear tracked aliases */
  break;
Index: vi.c
===================================================================
RCS file: /cvs/src/bin/ksh/vi.c,v
retrieving revision 1.28
diff -u -p -r1.28 vi.c
--- vi.c 18 Dec 2013 16:45:46 -0000 1.28
+++ vi.c 1 Sep 2015 00:22:04 -0000
@@ -1401,7 +1401,7 @@ static void
 free_edstate(struct edstate *old)
 {
  afree(old->cbuf, APERM);
- afree((char *)old, APERM);
+ afree(old, APERM);
 }
 
 

Reply | Threaded
Open this post in threaded view
|

Re: Like free(), ksh's afree() is NULL-safe

Ted Unangst-6
Michael McConville wrote:
> Ted Unangst wrote:
> > Michael McConville wrote:
> > > Also, why is fs on line 390 cast to char* when afree() takes void*?
> >
> > this code is older than void. and NULL apparently. please remove the casts
> > too.
>
> Does this look good? I had to leave one cast because the freed pointer
> is a const.

yup. only binary change is in two places, where the null check was removed.