Skip to content

Commit

Permalink
More cleanup (#185)
Browse files Browse the repository at this point in the history 
* Remove FREE_LINE_BUF macro
* Switch to using calloc() instead of malloc().
* Move temporary file creation in its own function.
* Better variable naming.
  • Loading branch information
@tjko
tjko authored Jan 27, 2025
1 parent 3772e5b commit a5c0bd2
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 71 deletions.
114 changes: 48 additions & 66 deletions jpegoptim.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,6 @@
#endif


#define FREE_LINE_BUF(buf,lines) { \
int j; \
for (j=0;j<lines;j++) free(buf[j]); \
free(buf); \
buf=NULL; \
}

struct my_error_mgr {
struct jpeg_error_mgr pub;
jmp_buf setjmp_buffer;
Expand Down Expand Up @@ -142,7 +135,7 @@ long average_count = 0;
double average_rate = 0.0;
double total_save = 0.0;

struct option long_options[] = {
const struct option long_options[] = {
#ifdef HAVE_ARITH_CODE
{ "all-arith", 0, &arith_mode, 1 },
{ "all-huffman", 0, &arith_mode, 0 },
Expand Down Expand Up @@ -197,6 +190,21 @@ struct option long_options[] = {

/*****************************************************************/


void free_line_buf(JSAMPARRAY *buf, unsigned int lines)
{
if (*buf == NULL)
return;

for (unsigned int i = 0; i < lines; i++) {
if ((*buf)[i])
free((*buf)[i]);
}
free(*buf);
*buf = NULL;
}


METHODDEF(void) my_error_exit (j_common_ptr cinfo)
{
my_error_ptr myerr = (my_error_ptr)cinfo->err;
Expand Down Expand Up @@ -575,10 +583,9 @@ unsigned int parse_markers(const struct jpeg_decompress_struct *dinfo,
int com_seen = 0;
int special;

if ((seen = malloc(marker_types)) == NULL)
if ((seen = calloc(marker_types, 1)) == NULL)
fatal("not enough of memory");

memset(seen, 0, marker_types);
str[0] = 0;
*markers_total_size = 0;

Expand Down Expand Up @@ -632,7 +639,6 @@ int optimize(FILE *log_fh, const char *filename, const char *newname,

long insize = 0, outsize = 0, lastsize = 0;
long inpos;
int j;
int oldquality, searchcount, searchdone;
double ratio;
int res = -1;
Expand Down Expand Up @@ -675,8 +681,7 @@ int optimize(FILE *log_fh, const char *filename, const char *newname,
abort_decompress:
jpeg_abort_decompress(&dinfo);
fclose(infile);
if (buf)
FREE_LINE_BUF(buf,dinfo.output_height);
free_line_buf(&buf, dinfo.output_height);
if (!quiet_mode || csv)
fprintf(log_fh,csv ? ",,,,,error\n" : " [ERROR]\n");
jderr.jump_set=0;
Expand All @@ -695,15 +700,14 @@ int optimize(FILE *log_fh, const char *filename, const char *newname,
if (stdin_mode || stdout_mode) {
if (inbuffer)
free(inbuffer);
inbuffersize=65536;
inbuffer=malloc(inbuffersize);
if (!inbuffer)
inbuffersize = 65536;
if (!(inbuffer=calloc(inbuffersize, 1)))
fatal("not enough memory");
}
global_error_counter=0;
jpeg_save_markers(&dinfo, JPEG_COM, 0xffff);
for (j=0;j<=15;j++) {
jpeg_save_markers(&dinfo, JPEG_APP0+j, 0xffff);
for (int i = 0; i < 16; i++) {
jpeg_save_markers(&dinfo, JPEG_APP0 + i, 0xffff);
}
jpeg_custom_src(&dinfo, infile, &inbuffer, &inbuffersize, &inbufferused, 65536);
jpeg_read_header(&dinfo, TRUE);
Expand Down Expand Up @@ -735,12 +739,11 @@ int optimize(FILE *log_fh, const char *filename, const char *newname,
jpeg_start_decompress(&dinfo);

/* Allocate line buffer to store the decompressed image */
buf = malloc(sizeof(JSAMPROW)*dinfo.output_height);
if (!buf) fatal("not enough memory");
for (j=0;j<dinfo.output_height;j++) {
buf[j]=malloc(sizeof(JSAMPLE)*dinfo.output_width*
dinfo.out_color_components);
if (!buf[j])
if (!(buf = calloc(dinfo.output_height, sizeof(JSAMPROW))))
fatal("not enough memory");
for (int i = 0; i < dinfo.output_height; i++) {
if (!(buf[i]=calloc((size_t)dinfo.output_width * dinfo.out_color_components,
sizeof(JSAMPLE))))
fatal("not enough memory");
}

Expand Down Expand Up @@ -790,13 +793,12 @@ int optimize(FILE *log_fh, const char *filename, const char *newname,

if (setjmp(jcerr.setjmp_buffer)) {
/* Error handler for compress failures */
if (!quiet_mode)
fprintf(log_fh," [Compress ERROR: %s]\n",last_error);
jpeg_abort_compress(&cinfo);
jpeg_abort_decompress(&dinfo);
fclose(infile);
if (!quiet_mode)
fprintf(log_fh," [Compress ERROR: %s]\n",last_error);
if (buf)
FREE_LINE_BUF(buf,dinfo.output_height);
free_line_buf(&buf, dinfo.output_height);
jcerr.jump_set=0;
res = 2;
goto exit_point;
Expand All @@ -819,9 +821,8 @@ int optimize(FILE *log_fh, const char *filename, const char *newname,
/* Allocate memory buffer that should be large enough to store the output JPEG... */
if (outbuffer)
free(outbuffer);
outbuffersize=insize + 32768;
outbuffer=malloc(outbuffersize);
if (!outbuffer)
outbuffersize = insize + 32768;
if (!(outbuffer=calloc(outbuffersize, 1)))
fatal("not enough memory");

/* setup custom "destination manager" for libjpeg to write to our buffer */
Expand Down Expand Up @@ -864,7 +865,6 @@ int optimize(FILE *log_fh, const char *filename, const char *newname,
cinfo.write_JFIF_header = FALSE;
}

j=0;
jpeg_start_compress(&cinfo,TRUE);

/* Write markers */
Expand Down Expand Up @@ -967,10 +967,9 @@ int optimize(FILE *log_fh, const char *filename, const char *newname,
}
}

if (buf)
FREE_LINE_BUF(buf,dinfo.output_height);
jpeg_finish_decompress(&dinfo);
fclose(infile);
free_line_buf(&buf, dinfo.output_height);


if (quality >= 0 && outsize >= insize && !retry && !stdin_mode) {
Expand Down Expand Up @@ -1010,7 +1009,7 @@ int optimize(FILE *log_fh, const char *filename, const char *newname,
int newlen = snprintf(tmpfilename, sizeof(tmpfilename),
"%s.jpegoptim.bak", newname);
if (newlen >= sizeof(tmpfilename))
warn("temp filename too long: %s", tmpfilename);
fatal("temp filename too long: %s", tmpfilename);

if (verbose_mode > 1 && !quiet_mode)
fprintf(log_fh,"%s, creating backup as: %s\n",
Expand All @@ -1024,30 +1023,11 @@ int optimize(FILE *log_fh, const char *filename, const char *newname,
if ((outfile=create_file(newname))==NULL)
fatal("%s, error opening output file: %s",
(stdin_mode ? "stdin" : filename), newname);
outfname=newname;
outfname = newname;
} else {
#ifdef HAVE_MKSTEMPS
/* rely on mkstemps() to create us temporary file safely... */
int newlen = snprintf(tmpfilename,sizeof(tmpfilename),
"%sjpegoptim-%d-%d.XXXXXX.tmp",
tmpdir, (int)getuid(), (int)getpid());
if (newlen >= sizeof(tmpfilename))
warn("temp filename too long: %s", tmpfilename);
int tmpfd = mkstemps(tmpfilename,4);
if (tmpfd < 0)
fatal("%s, error creating temp file %s: mkstemps() failed",
(stdin_mode ? "stdin" : filename), tmpfilename);
if ((outfile = fdopen(tmpfd,"wb")) == NULL)
#else
/* if platform is missing mkstemps(), try to create
at least somewhat "safe" temp file... */
snprintf(tmpfilename,sizeof(tmpfilename),
"%sjpegoptim-%d-%d.%ld.tmp", tmpdir,
(int)getuid(), (int)getpid(), (long)time(NULL));
if ((outfile = create_file(tmpfilename)) == NULL)
#endif
fatal("error opening temporary file: %s", tmpfilename);
outfname=tmpfilename;
if (!(outfile = create_temp_file(tmpdir, "jpegoptim", tmpfilename, sizeof(tmpfilename))))
fatal("error creating temporary file: %s", tmpfilename);
outfname = tmpfilename;
}

if (verbose_mode > 1 && !quiet_mode)
Expand Down Expand Up @@ -1225,20 +1205,20 @@ int main(int argc, char **argv)
char newname[MAXPATHLEN + 1], dest_path[MAXPATHLEN + 1];
char namebuf[MAXPATHLEN + 2];
const char *filename;
volatile int i;
int arg_idx;
int res;
double rate, saved;
FILE *log_fh;
#ifdef PARALLEL_PROCESSING
struct worker *w;
int pipe_fd[2];
pid_t pid;
int j;


/* Allocate table to keep track of child processes... */
if (!(workers = malloc(sizeof(struct worker) * MAX_WORKERS)))
if (!(workers = calloc(MAX_WORKERS, sizeof(struct worker))))
fatal("not enough memory");
for (i = 0; i < MAX_WORKERS; i++) {
for (int i = 0; i < MAX_WORKERS; i++) {
workers[i].pid = -1;
workers[i].read_pipe = -1;
}
Expand Down Expand Up @@ -1280,8 +1260,8 @@ int main(int argc, char **argv)
return (res == 0 ? 0 : 1);
}

i=(optind > 0 ? optind : 1);
if (files_from == NULL && argc <= i) {
arg_idx = (optind > 0 ? optind : 1);
if (files_from == NULL && argc <= arg_idx) {
if (!quiet_mode)
fprintf(stderr, PROGRAMNAME ": file argument(s) missing\n"
"Try '" PROGRAMNAME " --help' for more information.\n");
Expand All @@ -1296,7 +1276,7 @@ int main(int argc, char **argv)
break;
filename = namebuf;
} else {
filename = argv[i];
filename = argv[arg_idx];
}

if (*filename == 0)
Expand Down Expand Up @@ -1365,6 +1345,8 @@ int main(int argc, char **argv)
exit(res);
} else {
/* Parent continues here... */
int j;

close(pipe_fd[1]);

w = NULL;
Expand Down Expand Up @@ -1400,7 +1382,7 @@ int main(int argc, char **argv)
}
}

} while (files_from || ++i < argc);
} while (files_from || ++arg_idx < argc);


#ifdef PARALLEL_PROCESSING
Expand Down
1 change: 1 addition & 0 deletions jpegoptim.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ extern int quiet_mode;

/* misc.c */
FILE* create_file(const char *name);
FILE *create_temp_file(const char *tmpdir, const char *name, char *filename, size_t filename_len);
int delete_file(const char *name);
long file_size(FILE *fp);
int is_directory(const char *path);
Expand Down
51 changes: 46 additions & 5 deletions misc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include <string.h>
#include <stdarg.h>
#include <stdlib.h>
#include <time.h>


#include "jpegoptim.h"
Expand Down Expand Up @@ -62,6 +63,41 @@ FILE* create_file(const char *name)
return f;
}


FILE *create_temp_file(const char *tmpdir, const char *name, char *filename, size_t filename_len)
{
FILE *f;
int newlen;

#ifdef HAVE_MKSTEMPS
/* Rely on mkstemps() to create us temporary file safely... */
newlen = snprintf(filename, filename_len, "%s%s-%u-%u.XXXXXX.tmp",
tmpdir, name, getuid(), getpid());
#else
/* If platform is missing mkstemps(), try to create at least somewhat "safe" temp file... */
newlen = snprintf(filename, filename_len, "%s%s-%u-%u.%lu.tmp",
tmpdir, name, getuid(), getpid(), (unsigned long)time(NULL));
#endif
if (newlen >= filename_len) {
warn("temp filename too long: %s", filename);
return NULL;
}

#ifdef HAVE_MKSTEMPS
int tmpfd = mkstemps(filename, 4);
if (tmpfd < 0) {
warn("error creating temp file: mkstemps('%s', 4) failed", filename);
return NULL;
}
f = fdopen(tmpfd, "wb");
#else
f = create_file(filename);
#endif

return f;
}


int delete_file(const char *name)
{
int retval;
Expand Down Expand Up @@ -144,12 +180,12 @@ int rename_file(const char *old_path, const char *new_path)
}


#define COPY_BUF_SIZE (256*1024)
#define COPY_BUF_SIZE (256 * 1024)

int copy_file(const char *srcfile, const char *dstfile)
{
FILE *in,*out;
unsigned char buf[COPY_BUF_SIZE];
unsigned char *buf;
int r,w;
int err=0;

Expand All @@ -166,11 +202,14 @@ int copy_file(const char *srcfile, const char *dstfile)
return -3;
}

if (!(buf = calloc(COPY_BUF_SIZE, 1)))
fatal("out of memory");


do {
r=fread(buf,1,sizeof(buf),in);
r = fread(buf, 1, COPY_BUF_SIZE, in);
if (r > 0) {
w=fwrite(buf,1,r,out);
w = fwrite(buf, 1, r, out);
if (w != r) {
err=1;
warn("error writing to file: %s", dstfile);
Expand All @@ -179,14 +218,16 @@ int copy_file(const char *srcfile, const char *dstfile)
} else {
if (ferror(in)) {
err=2;
warn("error reading file: %s", srcfile);
warn("error reading from file: %s", srcfile);
break;
}
}
} while (!feof(in));

fclose(out);
fclose(in);
free(buf);

return err;
}

Expand Down

0 comments on commit a5c0bd2

Please sign in to comment.